web pages broken due to dynamic changes between position:static/relative not changing containing block for position:absolute inside

RESOLVED FIXED in Firefox 23

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mcsmurf, Assigned: spohl)

Tracking

({regression})

Trunk
mozilla25
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 unaffected, firefox23+ fixed, firefox24+ fixed, firefox25+ fixed)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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
(Reporter)

Comment 1

6 years ago
This regressed between 5976b9c673f8 and 0888e29c83a3 (yesterday's and today's build), changesets: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5976b9c673f8&tochange=0888e29c83a3
(Reporter)

Comment 2

6 years ago
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.
(Assignee)

Comment 4

6 years ago
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?
Flags: needinfo?(roc)
tracking-firefox25: --- → ?
So what is visible on http://www.arewefastyet.com/?a=b&view=regress is also this bug or another ?
(Reporter)

Comment 6

6 years ago
It's the same bug, I checked with mozilla-inbound builds.

Comment 7

6 years ago
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.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 895099
(Reporter)

Comment 9

6 years ago
(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.
(Assignee)

Comment 10

6 years ago
Created attachment 777360 [details] [diff] [review]
Patch

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-
(Assignee)

Comment 12

6 years ago
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 :-(.
Flags: needinfo?(roc)
(Assignee)

Comment 15

6 years ago
Created attachment 777478 [details] [diff] [review]
Revert assert fix from bug 868498

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 #777360 - Attachment is obsolete: true
Attachment #777478 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
status-firefox22: --- → unaffected
status-firefox23: --- → affected
status-firefox24: --- → affected
status-firefox25: --- → affected
tracking-firefox23: --- → ?
(Assignee)

Updated

6 years ago
Blocks: 895203
(Assignee)

Comment 17

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f713a828fe36
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
tracking-firefox24: --- → ?
(Assignee)

Comment 18

6 years ago
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
Attachment #777478 - Flags: approval-mozilla-beta?
Attachment #777478 - Flags: approval-mozilla-aurora?
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.
Attachment #777478 - Flags: approval-mozilla-beta?
Attachment #777478 - Flags: approval-mozilla-beta+
Attachment #777478 - Flags: approval-mozilla-aurora?
Attachment #777478 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 20

6 years ago
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...
Flags: needinfo?(dbaron)
(Assignee)

Comment 22

6 years ago
(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.
Flags: needinfo?(dbaron)
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
Last Resolved: 6 years ago
status-firefox23: affected → fixed
status-firefox24: affected → fixed
status-firefox25: affected → fixed
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
tracking-firefox23: ? → +
tracking-firefox24: ? → +
tracking-firefox25: ? → +
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...
Flags: in-testsuite?
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.

Updated

6 years ago
Duplicate of this bug: 895453
(Assignee)

Comment 32

6 years ago
Test pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/be10b6e8dfad
Flags: in-testsuite? → in-testsuite+

Updated

6 years ago
Duplicate of this bug: 896149

Updated

6 years ago
No longer blocks: 895203
Depends on: 895203
You need to log in before you can comment on or make changes to this bug.