Closed Bug 786672 Opened 7 years ago Closed 7 years ago

Position: fixed elements seem to be broken in Nightly

Categories

(Core :: Layout, defect)

17 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox17 --- fixed
firefox18 --- verified

People

(Reporter: lucasr, Assigned: cwiiis)

References

Details

(Keywords: regression)

Attachments

(2 files)

The Reader toolbar is a position: fixed element with bottom: 0px. In Beta (FF16) and Aurora (FF17) the toolbar stays on the same position while panning content (as expected). On latest Nightly though, the toolbar moves while panning.

Need to find the regression range. Seems like a serious regression.
Seems to work now on latest nightly. Never mind.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
In case this comes up again, to help bisect, a bad revision is 103641:271ca35d7645.

I had a quick look at the layer tree, and async scrolling of fixed pos elements breaks because basically the entire layer tree is marked as fixed position, including the root scroll layer. This could indicate that either nsLayoutUtils::IsScrolledByRootContentDocumentDisplayportScrolling has been changed, nsLayoutUtils::GetActiveScrolledRootFor has changed, or the frame-tree has changed in some was so as to affect one or both of those two functions.

The generated display-list is fine and correct.
Oh dear, so after stating that this wasn't my fault, I find that this is indeed my fault - This is caused by bug 785333, which just landed again. Looking into it.
Blocks: 785333
Status: RESOLVED → REOPENED
QA Contact: chrislord.net
Resolution: WORKSFORME → ---
I think this may have been caused by removing the code that swapped the underlying frame when merging in nsDisplayScrollLayer. Testing now.
This fixes the issue and I'm attaching it in case this needs to be urgently fixed, or developers would like to have this feature working for the time being.

I'm going to look into why this happens and see if there's a more reliable way of working around it.
And here's a real patch to fix it.

I think there may be a similar issue with other item types - I still see fixed-background breaking when zooming in (but this happened before any of these patches too), so it might be nice to find a more generic way of fixing this in all situations... Beats me what that way would be though, for now.
Attachment #656547 - Flags: review?(roc)
(In reply to Chris Lord [:cwiiis] from comment #6)
> Created attachment 656547 [details] [diff] [review]
> Fix getting the active scrolled root of an nsDisplayScrollLayer
> 
> And here's a real patch to fix it.
> 
> I think there may be a similar issue with other item types - I still see
> fixed-background breaking when zooming in (but this happened before any of
> these patches too), so it might be nice to find a more generic way of fixing
> this in all situations... Beats me what that way would be though, for now.

To illustrate the background issue, you can go to http://moonintern.com/, scroll while fully zoomed out, then zoom in slightly (but make sure the background is still visible) and then scroll. I see the same thing on various other sites too.
https://hg.mozilla.org/mozilla-central/rev/6a95de502ec1
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
This issue is not reproducible anymore on the latest Nightly build. Closing as verified fixed on:

Firefox 18.0a1 (2012-08-31)
Device: Galaxy Note
OS: Android 4.0.4
Status: RESOLVED → VERIFIED
Assignee: nobody → chrislord.net
Component: Graphics, Panning and Zooming → Layout
Keywords: regression
Product: Firefox for Android → Core
QA Contact: chrislord.net
Hardware: All → ARM
Target Milestone: Firefox 18 → mozilla18
Version: Trunk → 18 Branch
Comment on attachment 656547 [details] [diff] [review]
Fix getting the active scrolled root of an nsDisplayScrollLayer

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Merging bug 785333 but not this will break position:fixed async scrolling
User impact if declined: Async scrolling of position:fixed elements will often break
Testing completed (on m-c, etc.): Baked on m-c for a while, no complaints.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None.
Attachment #656547 - Flags: approval-mozilla-aurora?
Comment on attachment 656547 [details] [diff] [review]
Fix getting the active scrolled root of an nsDisplayScrollLayer

needed for 785333, approving.
Attachment #656547 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Version: 18 Branch → 17 Branch
You need to log in before you can comment on or make changes to this bug.