Closed Bug 1063494 Opened 6 years ago Closed 6 years ago

Fixed position header in Gaia messaging app does not stay fixed with async scrolling

Categories

(Core :: Graphics: Layers, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox33 --- unaffected
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: cwiiis, Assigned: kats)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

STR:

1- Open up a messaging thread long enough to scroll (may need to span more than a day, not sure)
2- Scroll up and down

Expected:

The view scrolls normally.

Actual:

The fixed-position date bar at the top jitters about during scrolling

This is almost certainly fallout from multi-layer apzc, so I'm going to block that, but adding regressionwindow-wanted just in case.
Attached file Layers dump
I'm able to reproduce this on a flame running latest master. I used |make reference workload-medium| and opened up one of the SMS threads that gets populated. It looks like the fixed-position layer isn't getting aligned in AsyncCompositionManager and so it only moves on repaints. Layers dump attached.
This is probably happening because the layer with isStickyPosition set on it is the same layer that has the scrollable metrics. So yeah, almost certainly a regression from multi-layer-apz, but we should be able to fix it in AsyncCompositionManager, I think.
Attached patch Patch (obsolete) — Splinter Review
This fixes it. If I don't take out the clip rect bit then the date row partially disappears while scrolling. I don't understand why the clip rect is shifted at all, it seems to me like if we're not adjusting the clip rect for async scroll then we shouldn't be adjusting it for fixed layers either.
Assignee: nobody → bugmail.mozilla
Attachment #8485775 - Flags: review?(botond)
Attachment #8485775 - Flags: review?(bgirard)
Comment on attachment 8485775 [details] [diff] [review]
Patch

Actually after thinking about it some more I think the patch might be wrong. I'll see if I can write some tests to exercise this.
Attachment #8485775 - Flags: review?(botond)
Attachment #8485775 - Flags: review?(bgirard)
Attached patch Patch v2Splinter Review
Ok, this patch I think is correct.

Try push with just the sticky-pos test showing that it fails: https://tbpl.mozilla.org/?tree=Try&rev=5461ecde83f8
Try push with the full patch showing that it passes:
https://tbpl.mozilla.org/?tree=Try&rev=35895c90f1e0

The fixed-pos test would have failed on the previous version (I verified that locally, but didn't do a try run) of the patch but passes with this version.
Attachment #8485775 - Attachment is obsolete: true
Attachment #8486363 - Flags: review?(botond)
Attachment #8486363 - Flags: review?(bgirard)
Comment on attachment 8486363 [details] [diff] [review]
Patch v2

Not that my opinion matters too much on this now, but this looks good to me :)
Attachment #8486363 - Flags: feedback+
Attachment #8486363 - Flags: review?(botond) → review+
Attachment #8486363 - Flags: review?(bgirard) → review+
https://hg.mozilla.org/mozilla-central/rev/ec8a228486a8
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8486363 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: bug 967844
[User impact if declined]: some pieces of content that are position:fixed or position:sticky jitter while scrolling
[Describe test coverage new/current, TBPL]: added reftests, manual testing
[Risks and why]: low risk, code is pretty well understood. only affects platforms with async scrolling (b2g and fennec)
[String/UUID change made/needed]: none
Attachment #8486363 - Flags: approval-mozilla-aurora?
Attachment #8486363 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This issue has been verified successfully on Flame2.1&2.2.
Reproducing rate: 0/5
See attachment: Verify_Flame_FTE.png

Flame2.1 build version:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0

Flame2.2 build version:
Gaia-Rev        725685831f5336cf007e36d9a812aad689604695
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2c9781c3e9b5
Build-ID        20141203040207
Version         37.0a1
Attached video Verify_Flame_SMS.MP4
You need to log in before you can comment on or make changes to this bug.