Closed
Bug 1513908
Opened 5 years ago
Closed 5 years ago
The css property will-change is breaking fixed layouts in firefox 64
Categories
(Core :: Layout: Flexbox, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: shyamzpassion, Unassigned)
References
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36 Steps to reproduce: Added "will-change:transform" to an overlay element (position:fixed) that transitions to viewport based on transform property Actual results: Causes unexpected behaviour -the overlay starts scrolling. This happens only in firefox 64 and not in older versions of firefox or in other browsers Expected results: Fixed element behaviour should not get affected by will-change
Comment 1•5 years ago
|
||
Thanks for the report. Could you attach a testcase (or post a link to a sample affected page) that demonstrates the issue?
Flags: needinfo?(shyamzpassion)
Reporter | ||
Comment 2•5 years ago
|
||
Our website runs behind a login session. So for your replication, I have hosted a static snapshot of the DOM here: https://zylker.online/session/firefoxBug/. If you open this in chrome there wouldn't be a scrollbar. But in firefox 64 the page would scroll. The layout gets fixed in firefox 64 if will-change property is removed. Please get back to me if you need anything else.
Flags: needinfo?(shyamzpassion)
Comment 3•5 years ago
|
||
Thanks for the test-case, I'll find the regression range, but FWIW the assumption is wrong. will-change: transform does establish a containing block for fixed-position descendants, just like transform: scale(1) does.
Comment 4•5 years ago
|
||
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7f9d03c29a6ffd82c1b5e17c14e27a2ae9d64434&tochange=b77bde54527692f87c31a60112d3cb57ec13298e Yeah, so bug 1498873 "broke" this by making the flex container properly a fixed containing block. Before that you were relying on a Firefox bug. Indeed if you change z-root element to be display: block, you can see the broken behavior. Now there's the question of why Chrome, which also implements this behavior of making will-change: transform items fixed containing blocks doesn't show the scrollbar... Looking into that now.
Blocks: 1498873
Comment 5•5 years ago
|
||
So, this is a flexbox interop issue. The reason Chrome doesn't show the scrollbars is because the element with id="ember1702" is never taller than the viewport, while in Firefox it is. Adding min-height: 0 to that element in Firefox makes our behavior match Chrome's (and thus not show the scrollbar). This is a Chrome bug that was fixed already, actually, and Chrome Canary shows the same behavior as Firefox 64, so it'll break in future versions of Chrome as well. So this bug is invalid (though it was fun to investigate! :)). Reporter, mind confirming that and adding that rule to fix your site on Firefox and Chrome Canary? ni? Daniel just to double-check, but I'm moderately sure the above is right.
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Component: Layout: Positioned → Layout: Flexbox
Flags: needinfo?(shyamzpassion)
Flags: needinfo?(dholbert)
Resolution: --- → INVALID
Reporter | ||
Comment 6•5 years ago
|
||
Thanks and really appreciate the swift response. I have made the foll. changes as per your suggestions and hosted the static snapshot @ https://zylker.online/session/firefoxBug_afterProposedChanges/ . But the same issue still persists. The layout gets fixed in firefox 64 only when will-change is removed. 1. I have made z-root a block level element (eliminating flexbox from this scenario to avoid mis-diagnosis) 2. I have also added min-height:0 as per your suggestion. Eagerly awaiting your response P.S. Sorry for mentioning a comparison with Chrome alone. What I actually should have implied was that this layout works fine in every other browser: Edge, Safari, Chrome (I'm yet to check the chrome canary build though)
Flags: needinfo?(shyamzpassion)
Comment 7•5 years ago
|
||
Sorry, should've been clearer. The block change is not needed (is indeed counter-productive, since it'll make the min-height: 0 change useless actually). The only change needed to fix the layout everywhere including Chrome Canary is adding min-height: 0 to the <div id="ember1702"> on your first example (id="ember1735" on your second example). _not_ on z-root. Does that make sense?
Flags: needinfo?(shyamzpassion)
Comment 8•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #7) > Sorry, should've been clearer. The block change is not needed (is indeed > counter-productive, since it'll make the min-height: 0 change useless > actually). To clarify: the block change was to prove that this behavior was really present for blocks before my patch, but it's not any sort of fix.
Reporter | ||
Comment 9•5 years ago
|
||
Oh ok. I have added min-height:0 only to the particular div you had mentioned and hosted the static snapshot @ https://zylker.online/session/firefoxBug_afterProposedChanges/ . The issue still persists. Though, removing will-change:transform seems to fix the issue.
Flags: needinfo?(shyamzpassion)
Comment 10•5 years ago
|
||
You didn't seem to change the element that I was pointing out. Let me attach an screenshot.
Comment 11•5 years ago
|
||
Just for reference, this is the element you should apply min-height: 0 to.
Comment 12•5 years ago
|
||
You can see how it fixes the issue here.
Reporter | ||
Comment 13•5 years ago
|
||
Th(In reply to Emilio Cobos Álvarez (:emilio) from comment #12) > Created attachment 9031428 [details] > Element with the proposed change. > > You can see how it fixes the issue here. Thanks a million!! This fixes the problem for Firefox 64 as well as Chrome Canary. But, just curious, is this is the intended behaviour as per W3C for the cases involving a position fixed child inside a flex parent? Can you tell me what behavioural change was brought about in the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1498873?
Comment 14•5 years ago
|
||
(In reply to shyamzpassion from comment #13) > Th(In reply to Emilio Cobos Álvarez (:emilio) from comment #12) > > Created attachment 9031428 [details] > > Element with the proposed change. > > > > You can see how it fixes the issue here. > > Thanks a million!! This fixes the problem for Firefox 64 as well as Chrome > Canary. > But, just curious, is this is the intended behaviour as per W3C for > the cases involving a position fixed child inside a flex parent? Can you > tell me what behavioural change was brought about in the fix for > https://bugzilla.mozilla.org/show_bug.cgi?id=1498873? You can see the behavior change here: https://crisal.io/tmp/overflow-fixed-pos-cb.html All six squares are supposed to render the same. Firefox was failing to parent the fixed item correctly to the will-change flex container, so it wasn't taken into account for overflow computation, which means that no scrollbar appeared, and the fixed element wasn't properly clipped either. That's what bug 1498873 fixed. The other thing that happened in parallel was a bug in Chrome's flexbox implementation (that Safari still has I suspect) that made it work correctly in Chrome by accident.
Updated•5 years ago
|
Flags: needinfo?(dholbert)
Comment 15•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5) > ni? Daniel just to double-check, but I'm moderately sure the above is right. Your diagnosis sounds correct, yeah. Thanks for looking into this!
Reporter | ||
Comment 16•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #14) > (In reply to shyamzpassion from comment #13) > > Th(In reply to Emilio Cobos Álvarez (:emilio) from comment #12) > > > Created attachment 9031428 [details] > > > Element with the proposed change. > > > > > > You can see how it fixes the issue here. > > > > Thanks a million!! This fixes the problem for Firefox 64 as well as Chrome > > Canary. > > But, just curious, is this is the intended behaviour as per W3C for > > the cases involving a position fixed child inside a flex parent? Can you > > tell me what behavioural change was brought about in the fix for > > https://bugzilla.mozilla.org/show_bug.cgi?id=1498873? > > You can see the behavior change here: > https://crisal.io/tmp/overflow-fixed-pos-cb.html > > All six squares are supposed to render the same. Firefox was failing to > parent the fixed item correctly to the will-change flex container, so it > wasn't taken into account for overflow computation, which means that no > scrollbar appeared, and the fixed element wasn't properly clipped either. > > That's what bug 1498873 fixed. The other thing that happened in parallel was > a bug in Chrome's flexbox implementation (that Safari still has I suspect) > that made it work correctly in Chrome by accident. Thanks for the info! Currently Firefox 64 seems to be the only browser to render https://crisal.io/tmp/overflow-fixed-pos-cb.html correctly. Kudos!!! Chrome has also come up with the fix but its still in Canary. But I still have specific doubts reg our bug. 1. According to W3C standards, fixed elements are supposed to be rendered with respect to the viewport right? shouldn't the fixed element be undisturbed by the parent (will-change flex container)? 2. From an abstract perspective adding min-height:0 looks like a hack. Under-the-hood, how exactly is adding min-height:0 fixing this?
Reporter | ||
Comment 17•5 years ago
•
|
||
(In reply to shyamzpassion from comment #16) > 1. According to W3C standards, fixed elements are supposed to be rendered > with respect to the viewport right? shouldn't the fixed element be > undisturbed by the parent (will-change flex container)? "As it is out-of-flow, an absolutely-positioned child of a flex container does not participate in flex layout. The static position of an absolutely-positioned child of a flex container is determined such that the child is positioned as if it were the sole flex item in the flex container, assuming both the child and the flex container were fixed-size boxes of their used size" These lines are quoted from https://www.w3.org/TR/css-flexbox-1/#abspos-items I assume the above quote is valid for our context since W3C also says "Fixed positioning is similar to absolute positioning. The only difference is that for a fixed positioned box, the containing block is established by the viewport."
Comment 18•5 years ago
|
||
(In reply to shyamzpassion from comment #17) > I assume the above quote is valid for our context since W3C also says "Fixed > positioning is similar to absolute positioning. The only difference is that > for a fixed positioned box, the containing block is established by the > viewport." Looks like this ^^ is a quote from https://www.w3.org/TR/CSS2/visuren.html#fixed-positioning That's superceded by text in more-recent & more-specific specs -- in this case, CSS Transforms, which says: "For elements whose layout is governed by the CSS box model, any value other than "none" for the transform property also causes the element to establish a containing block for all descendants. Its padding box will be used to layout for all of its [...] fixed-position descendants" And the "will-change" spec text says that if any value for a property can have this sort of effect, then "will-change: [that-property-name]" has that same effect as well.
Comment 19•5 years ago
|
||
That transform spec quote is from the bottom of https://www.w3.org/TR/2018/WD-css-transforms-1-20181130/#transform-rendering And the relevant "will-change" spec text is here: "If any non-initial value of a property would cause the element to generate a containing block for fixed-position elements, specifying that property in will-change must cause the element to generate a containing block for fixed-position elements." https://www.w3.org/TR/css-will-change-1/#will-change
Comment 20•5 years ago
•
|
||
Replying to your other question from comment #17: > 2. From an abstract perspective adding min-height:0 looks like a hack. > Under-the-hood, how exactly is adding min-height:0 fixing this? See my explanation in bug 1212901 comment 10 -- it's basically the same as I laid out there, I think. (Very briefly: there's a default "min-height:auto" value for flex items which has an automagic content-based sizing behavior, which in your case makes the item refuse to shrink smaller than the tall content in its descendant. You can get rid of this by explicitly telling flexbox to let this element shrink smaller than its content size. This is a reasonable thing to do, since you know the element can shrink because its tall content is inside of an "overflow:hidden" thing and so won't overflow the flex container.) (There some subtlety because min-height:auto has special resolving-to-zero behavior **for a flex item which is also styled as overflow:hidden**. But in this case, that special case doesn't apply -- the element that emilio pointed out does *not* have overflow:hidden itself. Its child does, but it does not. So it resolves min-height:auto to something tall, which is unfortunate. This is a wart of the flexbox spec design which lots of people have tripped over, in part because Chrome had a bug there that made it sometimes appear to Just Work. The CSSWG has a proposal to address this in https://github.com/w3c/csswg-drafts/issues/1865#issuecomment-347631894 but it's still in progress I think.)
Reporter | ||
Comment 21•5 years ago
|
||
Thanks for clearing my doubts, Daniel. Really appreciate the detailing and the instant responses!!
You need to log in
before you can comment on or make changes to this bug.
Description
•