-
Notifications
You must be signed in to change notification settings - Fork 657
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
[css-overflow] Overflow propagation when the element propagated from is display: none #3779
Comments
Basically, my point is that this should be closer to the background style propagation, which only happens if the document element is actually displayed. |
I think from an author standpoint this case doesn't matter, so personally I don't care. Happy to do whatever is expedient for implementers. |
Fine for us from an implementation pov. |
Chrome even propagates the background, which is not according to spec. |
For backgrounds: https://bugs.chromium.org/p/chromium/issues/detail?id=947873 |
There is the same issue for https://drafts.csswg.org/css-writing-modes-3/#principal-flow Chrome starts with the horizontal scrollbar to the right for direction:rtl regardless of display:none or not on body below.
|
…. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed w3c/csswg-drafts#3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455 --HG-- extra : moz-landing-system : lando
…. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed w3c/csswg-drafts#3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: Overflow propagation when the element propagated from is display: none<dael> github: https://github.com//issues/3779 <dael> plinss: Who wants this one? <dael> fantasai: Was emilio able to join? <dael> emilio: Hey <dael> emilio: This is mostly a little thing and only place where gecko needs to deep dive into the subtree to render <dael> emilio: I remember rune at some point complained about it. I was wondering if people had willingness to change. I don't deeply care but I think it's a bitsaner <dael> AmeliaBR: You're asking if the HTML or Body element isn't displayed we don't worry about prop. styles up? <dael> emilio: Yes <dael> plinss: Just get a blank page? <bkardell_> seems fine? <dael> smfr: display:none is common to do? <dael> emilio: I dont think it is. I could get some data and come back <dael> smfr: My only concerns is sites where they do that and browsers prop. the body background and you get a flash of color change <dael> emilio: Body background is spec to not prop if there's display:none <dael> smfr: I guess Chrome has a bug that it doesn't do that which is shared by WK <dael> emilio: Def Chrome, I don't know WK <dael> smfr: Prob does. I don't feel too strongly. Seems reasonable <dael> plinss: Curiousabout usage of display:none body. I can see it abused for tracking iframes and whatnot on a page. Curious if that's something people want to do.I guess if they're putting in iframe they won't put a bg on it <bkardell_> but changing -- there isn't interop now -- or is there? <dael> plinss: Prop: Not prop any style from display:none HTML or Body <dael> emilio: Don't propagate the background of the element...basically whatever definition the background propagation is using I want to align <dael> plinss: Objections? <dael> RESOLVED: Do not propagate any style from display:none HTML or Body |
When documentElement/body doe not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834}
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834}
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834}
…pagation., a=testonly Automatic update from web-platform-tests Handle display:contents for viewport propagation. When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834} -- wpt-commits: d40f18ae072cab633cfb34fb883e12c87c1bc191 wpt-pr: 18039
…pagation., a=testonly Automatic update from web-platform-tests Handle display:contents for viewport propagation. When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834} -- wpt-commits: d40f18ae072cab633cfb34fb883e12c87c1bc191 wpt-pr: 18039
…isplay: contents elements. r=dholbert Resolution is at: * w3c/csswg-drafts#3779 (comment) Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974, I'll make sure to wait until they're in the tree and ensure they're passing. Note that we need to ensure in the frame constructor that the document element is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves duplicated work (since ResolveStyleLazily throws away the style). ResolveStyleLazily already returns out of date styles, unless the element is not styled yet, or is in a `display: none` subtree, in which case it computes and returns a new (up-to-date) style. So the switch to the FFI call only should change behavior for the display: none subtree case (since we ensure the unstyled case isn't hit by styling the document earlier). But that case is one of the cases we're interested in changing, conveniently. Depends on D40080 Differential Revision: https://phabricator.services.mozilla.com/D39204 --HG-- extra : moz-landing-system : lando
…isplay: contents elements. r=dholbert Resolution is at: * w3c/csswg-drafts#3779 (comment) Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974, I'll make sure to wait until they're in the tree and ensure they're passing. Note that we need to ensure in the frame constructor that the document element is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves duplicated work (since ResolveStyleLazily throws away the style). ResolveStyleLazily already returns out of date styles, unless the element is not styled yet, or is in a `display: none` subtree, in which case it computes and returns a new (up-to-date) style. So the switch to the FFI call only should change behavior for the display: none subtree case (since we ensure the unstyled case isn't hit by styling the document earlier). But that case is one of the cases we're interested in changing, conveniently. Depends on D40080 Differential Revision: https://phabricator.services.mozilla.com/D39204
When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#680834}
…. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed w3c/csswg-drafts#3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455 UltraBlame original commit: ae9783be99324502017f3c9bfc92cb811fd1993e
…. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed w3c/csswg-drafts#3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455 UltraBlame original commit: ae9783be99324502017f3c9bfc92cb811fd1993e
…pagation., a=testonly Automatic update from web-platform-tests Handle display:contents for viewport propagation. When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futharkchromium.org> Reviewed-by: Anders Hartvoll Ruud <andruudchromium.org> Cr-Commit-Position: refs/heads/master{#680834} -- wpt-commits: d40f18ae072cab633cfb34fb883e12c87c1bc191 wpt-pr: 18039 UltraBlame original commit: b0a2ce5298cfff788febf0b952db102419a4753b
…isplay: contents elements. r=dholbert Resolution is at: * w3c/csswg-drafts#3779 (comment) Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974, I'll make sure to wait until they're in the tree and ensure they're passing. Note that we need to ensure in the frame constructor that the document element is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves duplicated work (since ResolveStyleLazily throws away the style). ResolveStyleLazily already returns out of date styles, unless the element is not styled yet, or is in a `display: none` subtree, in which case it computes and returns a new (up-to-date) style. So the switch to the FFI call only should change behavior for the display: none subtree case (since we ensure the unstyled case isn't hit by styling the document earlier). But that case is one of the cases we're interested in changing, conveniently. Depends on D40080 Differential Revision: https://phabricator.services.mozilla.com/D39204 UltraBlame original commit: 400b06aac33e27a53b1ff0da585cd57035661d4c
…. r=dholbert This switches nsFrameSetFrame's hacky frame construction codepath to operate on the flattened tree, since it made me a bit more comfortable about it (all layout should operate on is the flattened tree, though in this cause this should not cause any web-observable behavior change, since <frameset> can't be a shadow host per spec, and we no longer support XBL-in-content). That doesn't need to compute styles lazily. You only need to compute styles lazily in descendants of display: none elements, and even though this code doesn't check on display: none _on the children_, the parent element should never have display: none (since we're creating an nsFrameSetFrame for it). There are other places that still call into this (apart from getComputedStyle). One is nsImageFrame's cursor code. Given the <area> elements referencing an image map could be anywhere, we need to have lazy computation here. The other is the viewport style propagation stuff. There shouldn't be a need for LazyComputeBehavior::Allow on the document element in order to check the viewport styles propagation, since the root element can't have display: none ancestors. But we run that code before actually constructing the doc element containing block, which is when we do the initial document styling. We could remove that with some more effort, but it's not worth it right now, since we need to keep using it for the <body>, since the document element could be display: none itself, and we propagate the overflow styles in that case still. I filed w3c/csswg-drafts#3779 to potentially change that. Differential Revision: https://phabricator.services.mozilla.com/D25455 UltraBlame original commit: ae9783be99324502017f3c9bfc92cb811fd1993e
…pagation., a=testonly Automatic update from web-platform-tests Handle display:contents for viewport propagation. When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futharkchromium.org> Reviewed-by: Anders Hartvoll Ruud <andruudchromium.org> Cr-Commit-Position: refs/heads/master{#680834} -- wpt-commits: d40f18ae072cab633cfb34fb883e12c87c1bc191 wpt-pr: 18039 UltraBlame original commit: b0a2ce5298cfff788febf0b952db102419a4753b
…pagation., a=testonly Automatic update from web-platform-tests Handle display:contents for viewport propagation. When documentElement/body does not generate a box, its background will not propagate to the viewport (see [1]). This removes the need for calling EnsureComputedStyle() on documentElement/body. The CSSWG has also resolved that to be the case also for other properties propagated to the viewport[2]. Some of the wpt tests for body propagation didn't actually have a body element. Added. [1] https://drafts.csswg.org/css-backgrounds/#special-backgrounds [2] w3c/csswg-drafts#3779 (comment) Bug: 987207 Change-Id: I06e618e2acd2926b5ae4831bf5825e13e970d035 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1547974 Commit-Queue: Rune Lillesveen <futharkchromium.org> Reviewed-by: Anders Hartvoll Ruud <andruudchromium.org> Cr-Commit-Position: refs/heads/master{#680834} -- wpt-commits: d40f18ae072cab633cfb34fb883e12c87c1bc191 wpt-pr: 18039 UltraBlame original commit: b0a2ce5298cfff788febf0b952db102419a4753b
…isplay: contents elements. r=dholbert Resolution is at: * w3c/csswg-drafts#3779 (comment) Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974, I'll make sure to wait until they're in the tree and ensure they're passing. Note that we need to ensure in the frame constructor that the document element is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves duplicated work (since ResolveStyleLazily throws away the style). ResolveStyleLazily already returns out of date styles, unless the element is not styled yet, or is in a `display: none` subtree, in which case it computes and returns a new (up-to-date) style. So the switch to the FFI call only should change behavior for the display: none subtree case (since we ensure the unstyled case isn't hit by styling the document earlier). But that case is one of the cases we're interested in changing, conveniently. Depends on D40080 Differential Revision: https://phabricator.services.mozilla.com/D39204 UltraBlame original commit: 400b06aac33e27a53b1ff0da585cd57035661d4c
…isplay: contents elements. r=dholbert Resolution is at: * w3c/csswg-drafts#3779 (comment) Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974, I'll make sure to wait until they're in the tree and ensure they're passing. Note that we need to ensure in the frame constructor that the document element is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves duplicated work (since ResolveStyleLazily throws away the style). ResolveStyleLazily already returns out of date styles, unless the element is not styled yet, or is in a `display: none` subtree, in which case it computes and returns a new (up-to-date) style. So the switch to the FFI call only should change behavior for the display: none subtree case (since we ensure the unstyled case isn't hit by styling the document earlier). But that case is one of the cases we're interested in changing, conveniently. Depends on D40080 Differential Revision: https://phabricator.services.mozilla.com/D39204 UltraBlame original commit: 400b06aac33e27a53b1ff0da585cd57035661d4c
…e from 'display: none' elements. Do propagate from 'display: contents' elements. #3779
https://drafts.csswg.org/css-overflow/#overflow-propagation says that some overflow properties should be propagated to the viewport from the html or body element:
However this happens regardless of the display value of the document element or the body. For example, the following examples show scrollbars:
data:text/html,<html style="display: none"><body style="overflow: scroll">
data:text/html,<html style="display: none; overflow: scroll">
This is the only case where Gecko needs to resolve a style in a
display: none
subtree for rendering (apart of getComputedStyle), and I find it somewhat silly.Seems enough of an edge-case that could probably be changed. I wonder if there would be interest from other vendors in doing this change?
cc @lilles
The text was updated successfully, but these errors were encountered: