[Twitter] The fixed position of content is broken (header occasionally disappears, or partly disappears)

RESOLVED FIXED in Firefox 30

Status

()

defect
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sarsenyev, Assigned: tnikkel)

Tracking

({regression})

unspecified
mozilla30
Points:
---

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox28 wontfix, firefox29 wontfix, firefox30 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(3 attachments)

Description:
When scrolling a twitter page, the fixed content appears broken

Repro Steps:
1) Updated Buri to BuildID: 20140219040204
2) From home screen, open "browser" and type in URL http://twitter.com
3) Log in into "Twitter"
4) Open any page and scroll down and up

Actual:
The fixed content disappears and appears when scrolling

Expected:
The fixed content stays fixed on the twitter page

Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140219040204
Gaia: ac06cfbd2baf6494ffbb668cc599e3892cd5e17b
Gecko: bf0e76f2a7d4
Version: 30.0a1
Firmware Version: v1.2-device

Notes:
This bug is follow up of bug 941050 

Repro frequency: 100%
See attached: video clip
Summary: [Twitter] The fixed position of content is broken → [Twitter] The fixed position of content is broken (header occasionally gets scrolled offscreen)
The issue reproduces on the latest 1.3

Device: Buri 1.3 MOZ
BuildID: 20140219004003
Gaia: a43904d9646109b48836d62f7aa51e499fbf4b2e
Gecko: 97922b6daad1
Version: 28.0
Firmware Version: v1.2-device.cfg
Followup bug to a regressing blocker. Blocks a partner app.
blocking-b2g: --- → 1.3?
Keywords: regression
Renaming the bug to reflect what's actually happening.

It looks like the fixed position content is probably falling outside of the display port and not getting rendered/only getting partially rendered.

This probably needs to be fixed in nsDisplayList. n? tn for comment (/fix? :))
Flags: needinfo?(tnikkel)
Summary: [Twitter] The fixed position of content is broken (header occasionally gets scrolled offscreen) → [Twitter] The fixed position of content is broken (header occasionally disappears, or partly disappears)
blocking-b2g: 1.3? → 1.3+
Is this on the app or the website? Comment 0 says website but comment 2 says app.
(Also, should it still be 1.3+ if it's on the website?)
it happens on both and web and the app
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> (Also, should it still be 1.3+ if it's on the website?)

when blocking this the understanding was it happens everywhere and basically this is a gecko regression and a follow-up from a blocker bug 941050. And given this is a top app/website it made a strong blocking case.
Priority: -- → P2
So far I've only seen the problem when scrolling very fast so my first guess is that the displayport is completely outside the page and so doesn't intersect the fixed position content.
A quick test confirms.. this happens when the displayport doesn't contain the fixed pos content. Setting the displayport heuristic to "center displayport" in the developer options makes this bug go away (although results in more checkerboarding).

Given that this isn't very easy to reproduce and that it's a very transient problem (only a couple of frames) that goes away on its own I don't think this is a very high priority. Fixing it also has a nontrivial risk of regression.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> A quick test confirms.. this happens when the displayport doesn't contain
> the fixed pos content. Setting the displayport heuristic to "center
> displayport" in the developer options makes this bug go away (although
> results in more checkerboarding).
> 
> Given that this isn't very easy to reproduce and that it's a very transient
> problem (only a couple of frames) that goes away on its own I don't think
> this is a very high priority. Fixing it also has a nontrivial risk of
> regression.

This is a blocker for a content partner in all target markets & affects a top mobile site. We've got no choice here but to fix this.
So I tried testing this myself for a point of comparison on a 2/13 1.3 build - I couldn't reproduce this. Two things to clarify here:

1. What exactly happened on 1.3? Can we get a video showing 1.3's behavior?
2. Are you able to reproduce this on a 2/13 1.3 build as a point of comparison?

Build I used to test:

Build ID: 20140213175711
Device: Sora
Version: 1.3
QA Contact: mvaughan
(In reply to Jason Smith [:jsmith] from comment #11)
> So I tried testing this myself for a point of comparison on a 2/13 1.3 build
> - I couldn't reproduce this. Two things to clarify here:
> 
> 1. What exactly happened on 1.3? Can we get a video showing 1.3's behavior?

On the 02/24/14 1.3, Twitter's header will partially to fully disappear for a moment or so, but the header will reappear after that. 

This issue will reproduce after quickly scrolling for a little bit and the rest of the webpage starts checkerboarding. 

> 2. Are you able to reproduce this on a 2/13 1.3 build as a point of
> comparison?

This issue does reproduce for me on the 02/13/14 1.3 build on a Buri. The issue is similar when comparing the 2/13 and the 2/24 1.3 builds.
Keywords: qawanted
(In reply to Matthew Vaughan from comment #12)
> Created attachment 8380970 [details]
> Behavior on 2/24 1.3 (header completely disappears for a moment)
> 
> (In reply to Jason Smith [:jsmith] from comment #11)
> > So I tried testing this myself for a point of comparison on a 2/13 1.3 build
> > - I couldn't reproduce this. Two things to clarify here:
> > 
> > 1. What exactly happened on 1.3? Can we get a video showing 1.3's behavior?
> 
> On the 02/24/14 1.3, Twitter's header will partially to fully disappear for
> a moment or so, but the header will reappear after that. 
> 
> This issue will reproduce after quickly scrolling for a little bit and the
> rest of the webpage starts checkerboarding. 
> 
> > 2. Are you able to reproduce this on a 2/13 1.3 build as a point of
> > comparison?
> 
> This issue does reproduce for me on the 02/13/14 1.3 build on a Buri. The
> issue is similar when comparing the 2/13 and the 2/24 1.3 builds.

This isn't the same bug. The bug here involves the fact that twitter header is blinking during scrolling. It's not a full window checkerboarding effect. Please check again.
Keywords: qawanted
Posted patch patchSplinter Review
I don't think the displayport is even relevant to postion: fixed content (it is relevant to background-attachment: fixed, at least how it is currently implemented). The displayport is only relevant to scrolling content. The only part of position fixed content we'll ever show is what's inside the viewport, or the scroll position clamping scroll port rect (which probably needs a better name now that it is doing a lot more than just that).

This patch fixes the issue for me on hamachi.
Assignee: nobody → tnikkel
Attachment #8381163 - Flags: review?(roc)
Flags: needinfo?(tnikkel)
Keywords: qawanted
I don't understand this bug and patch. Is the APZC making the displayport not contain the whole CSS viewport? That seems wrong.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> I don't understand this bug and patch. Is the APZC making the displayport
> not contain the whole CSS viewport? That seems wrong.

Yes, that is correct. If the velocity is high then the content we want to draw might be offset enough so that it doesn't cover the viewport (even if it's height is larger than the viewport height). The theory is that by the time the paint is done the scroll offset will have changed enough so that it does cover the part of the scrolled content that we want to composite to the screen.
Comment on attachment 8381163 [details] [diff] [review]
patch

Review of attachment 8381163 [details] [diff] [review]:
-----------------------------------------------------------------

Fly-by to say that the patch looks sound (though I'd probably not know if it wasn't...), but some commenting would be nice.
Does comment 16 answer your question?
Flags: needinfo?(roc)
Comment on attachment 8381163 [details] [diff] [review]
patch

Review of attachment 8381163 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/FrameLayerBuilder.cpp
@@ +1707,5 @@
> +    displayPort.MoveTo(0, 0);
> +    if (presShell->IsScrollPositionClampingScrollPortSizeSet()) {
> +      displayPort.SizeTo(presShell->GetScrollPositionClampingScrollPortSize());
> +    } else {
> +      displayPort.SizeTo(viewport->GetSize());

Change the name of "displayPort" to something else since it's not the displayport anymore.
Attachment #8381163 - Flags: review?(roc) → review+
(In reply to Chris Lord [:cwiiis] from comment #17)
> Fly-by to say that the patch looks sound (though I'd probably not know if it
> wasn't...), but some commenting would be nice.

Added some comments.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #19)
> Change the name of "displayPort" to something else since it's not the
> displayport anymore.

Called it fixedVisibleRect.

https://hg.mozilla.org/integration/b2g-inbound/rev/809aaff30752
Flags: needinfo?(roc)
https://hg.mozilla.org/mozilla-central/rev/809aaff30752
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Please request b2g28 uplift on this patch when you get a chance :)
Flags: needinfo?(tnikkel)
Comment on attachment 8381163 [details] [diff] [review]
patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 919144 added the code changed here
User impact if declined: scrolling pages with fixed position content will sometimes not draw the fixed position content when scrolling quickly
Testing completed: been on m-c for a bit
Risk to taking this patch (and alternatives if risky): should be pretty low
String or UUID changes made by this patch: none
Attachment #8381163 - Flags: approval-mozilla-b2g28?
Flags: needinfo?(tnikkel)
Attachment #8381163 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
You need to log in before you can comment on or make changes to this bug.