Closed Bug 894931 Opened 7 years ago Closed 7 years ago
web pages broken due to dynamic changes between position:static/relative not changing containing block for position:absolute inside
On trunk (Firefox) the text on a certain website flows out of the usual box and takes up the whole page width instead. In the latest Firefox this was still fine. I'm going to look for the regression range of this (must be a very recent regression). To reproduce: 1. Go to http://www.nordbayerischer-kurier.de/ 2. Observe how the text below the animated pictures/slideshow flows out of the box where the text should be in
This regressed between 5976b9c673f8 and 0888e29c83a3 (yesterday's and today's build), changesets: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5976b9c673f8&tochange=0888e29c83a3
Some further testing: Build from this revision http://hg.mozilla.org/mozilla-central/rev/5f79304fcefe is still fine, this one http://hg.mozilla.org/mozilla-central/rev/fd10ead17ace is broken => http://hg.mozilla.org/mozilla-central/rev/fd10ead17ace is to "blame", it's a birch to mozilla-central merge.
Got it: Regression between http://hg.mozilla.org/integration/mozilla-inbound/rev/3d33faf4d81f and http://hg.mozilla.org/integration/mozilla-inbound/rev/e8bf739addfa => Bug 868498
Component: General → Layout
roc, see bug 868498 comment 47. The assertion fix there is causing this regression. Would you happen to know a way to work around this?
So what is visible on http://www.arewefastyet.com/?a=b&view=regress is also this bug or another ?
It's the same bug, I checked with mozilla-inbound builds.
I believe Huffington Post is also affected by this bug. The "Most Popular" column is rendered on top of the logo, navigaton bar and the top story, making it impossible to click on some sections on the site.
(In reply to d.a. from comment #7) > I believe Huffington Post is also affected by this bug. The "Most Popular" > column is rendered on top of the logo, navigaton bar and the top story, > making it impossible to click on some sections on the site. You're right, I checked with the two m-i builds. IMHO quite questionable if this patch can stay on trunk/aurora/beta when there is no fast workaround for this.
Looks like this is a bigger issue than first thought. roc, I'm setting this patch to r? although you may have a better way to fix this. It turns out that we don't need to move the z-index rule. The position rule is all we need to get the correct behavior when switching scrollbar styles. The position rule seems to only affect overlay scrollbars, so maybe this is safe to do?
Attachment #777360 - Flags: review?(roc)
Comment on attachment 777360 [details] [diff] [review] Patch Review of attachment 777360 [details] [diff] [review]: ----------------------------------------------------------------- This patch isn't correct because it changes the z-ordering of scrollbars.
Attachment #777360 - Flags: review?(roc) → review-
Oh, I think I finally understand: you're saying by changing the position we're also changing z-ordering? I missed that.
I know what the bug is. The bug is that we need to reframe when transitioning from position:static to position:relative if there are position:absolute descendants, because they need to change their containing blocks to be the new frame.
I can't think of an easy, beta-safe fix right now. I think we're going to have to pull disappearing scrollbars from beta, sorry :-(.
Okay, thanks roc! Let's use this bug to revert the assert fix in m-c, aurora and beta. I'll file a separate bug to turn off overlay scrollbars in beta.
Attachment #777478 - Flags: review?(roc) → review+
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Comment on attachment 777478 [details] [diff] [review] Revert assert fix from bug 868498 [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 868498 User impact if declined: Page layout on many websites would be broken. Testing completed (on m-c, etc.): This is merely a backout of one of the patches in bug 868498. Verified locally that this fixes things. Risk to taking this patch (and alternatives if risky): very low String or IDL/UUID changes made by this patch: none
Comment on attachment 777478 [details] [diff] [review] Revert assert fix from bug 868498 You don't need approval to back out something that you incorrectly landed this morning.
Thanks dbaron! Do the tracking flags need to be updated for this to propagate to the right branches? It's getting late here and I won't be able to watch the tree...
(And dare I ask why you don't need to back out the *other* patch?)
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #21) > (And dare I ask why you don't need to back out the *other* patch?) Sure. That's because the patch got lumped in with bug 868498, but it was really fixing bug 870941. I've taken the opportunity to separate the two. I reopened bug 870941 to track progress on the proper fix. Hope that makes sense.
(In reply to Stephen Pohl [:spohl] from comment #20) > Thanks dbaron! Do the tracking flags need to be updated for this to > propagate to the right branches? It's getting late here and I won't be able > to watch the tree... The tracking flags should be updated when it gets backed out to reflect the current state. But tracking flags don't cause propagation to branches, they just show it.
https://hg.mozilla.org/mozilla-central/rev/625e6c220d30 https://hg.mozilla.org/releases/mozilla-aurora/rev/668caad64c7a https://hg.mozilla.org/releases/mozilla-beta/rev/e4f6d58191b9 If this breaks the tree in any way, please back out the other patch from bug 868498.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Summary: Text on website flows out of the usual box → web pages broken due to dynamic changes between position:static/relative not changing containing block for position:absolute inside
Target Milestone: --- → mozilla25
BTW, please tell me we have tests for this now? Kinda scary when something can break site layouts as bad as this did without our automated tests catching it...
Comment on attachment 778280 [details] [diff] [review] test Maybe use padding-top on the outer div to avoid having to worry about whether margin collapsing makes the test pass in the failure case anyway (given some future margin collapsing changes, at least)?
Attachment #778280 - Flags: review?(dbaron) → review+
Actually padding-top won't work since we need to move the border-box.
I don't think collapsing with the top margin could ever be eliminated, due to compatibility issues, so I'll leave it as-is.
Test pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/be10b6e8dfad
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.