Skip to content

[css-backgrounds-3][css-borders-4] Fixed definition for where box shadows apply #9296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

SebastianZ
Copy link
Contributor

This corrects the description for where box shadows are applied.

Fixes #9286

Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was expecting the behavior to be the principal box since that's the simple choice, but I have tried a table:

<!DOCTYPE html>
<table style="outline: 3px solid cyan; box-shadow: 0 0 0 6px magenta">
  <caption>Tablecaption</caption>
  <tr><td>cell</td></tr>
</table>

Both Firefox and Chromium apply the shadow to the table grid box, not to the principal table wrapper box.

So maybe it should be discussed whether this should change? Or say that it's the principal box unless otherwise defined by the relevant spec (https://www.w3.org/TR/CSS21/tables.html#model would imply table grid box, and seemingly https://drafts.csswg.org/css-tables/#global-style-overrides too)

I can't find it now, but I remember some discussion about defaulting properties on a table to apply to the principal wrapper box, with some exceptions that apply to the grid box. Now the default is the grid box, which seems a bit weird.

@@ -2599,6 +2601,8 @@ Changes since the 14 February 2024 Candidate Recommendation Snapshot</h3>
(<a href="https://github.com/w3c/csswg-drafts/issues/8496">Issue 8496</a>)
<li>Fixed an error in the Computed Value line of 'background-image'.
(<a href="https://github.com/w3c/csswg-drafts/issues/8604">Issue 8604</a>)
<li>Fixed the definition for where box shadows apply.
(<a href="https://github.com/w3c/csswg-drafts/issues/9286">Issue 9286</a>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the "Changes since the 14 February 2024 Candidate Recommendation Snapshot" which doesn't exist (should be 2023 instead).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether @fantasai's comment was related to this, though I went ahead and fixed this obvious mistake.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I was commenting on the actual spec text.

@fantasai
Copy link
Collaborator

@Loirooriol Can you drop that comment into the issue?

@@ -2504,7 +2505,8 @@ Layering, Layout, and Other Details</h4>
and the <i>inner shadows</i> of an element are drawn immediately above the background of that element
(below the borders and border image, if any).

<p>If an element has multiple boxes, all of them get drop shadows,
<p>Drop shadows are only applied to the [=principal box=].
If that box has multiple [=fragments=], then all of them get shadows,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect. How box-shadow is handled on fragments depends on box-decoration-break, see https://www.w3.org/TR/css-break-4/#break-decoration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this addressed in the next sentence?

but shadows are only drawn where borders would also be drawn;
see 'box-decoration-break'.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's not that we have multiple shadows some parts of which aren't drawn. box-decoration-break determines whether the entire box gets shadowed and then sliced or the fragments get shadowed. If you offset the shadow by more than the width of the first fragment and then blur it, for example, the first fragment won't have a shadow attached, but there will be a shadow as wide as the combined fragments, blurred as a single unit, attached to the second fragment.

Comment on lines 2507 to 2517
<p>If an element has multiple boxes, all of them get drop shadows,
<p>Drop shadows are only applied to the [=principal box=].
If that box has multiple [=fragments=], then all of them get shadows,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  <p>Unless otherwise specified, drop shadows are only applied to the [=principal box=].
  If the affected box has multiple fragments,
  the shadows are applied as specified in 'box-decoration-break'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@SebastianZ SebastianZ force-pushed the 9286-drop-shadows-applying-to-principal-box branch from 82e2238 to 7190233 Compare November 16, 2023 21:43
@SebastianZ SebastianZ requested a review from fantasai November 16, 2023 21:45
@SebastianZ SebastianZ force-pushed the 9286-drop-shadows-applying-to-principal-box branch from 7190233 to 4a0b8ab Compare November 20, 2023 20:31
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't add the "Unless otherwise specified" into css-background-3. There is a suggested wording from fantasai.

@SebastianZ SebastianZ force-pushed the 9286-drop-shadows-applying-to-principal-box branch from baacc5d to 9b6b48f Compare November 25, 2023 20:26
@SebastianZ
Copy link
Contributor Author

You didn't add the "Unless otherwise specified" into css-background-3. There is a suggested wording from fantasai.

I made up for that now.

Sebastian

@SebastianZ SebastianZ force-pushed the 9286-drop-shadows-applying-to-principal-box branch from f727a92 to befc123 Compare December 20, 2023 22:05
@SebastianZ
Copy link
Contributor Author

@Loirooriol @fantasai Can you please have another look?

Sebastian

@SebastianZ SebastianZ merged commit aaa2783 into w3c:main Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[css-backgrounds-3][css-borders-4] Only the principal box should get a shadow
3 participants