Closed Bug 909525 Opened 11 years ago Closed 11 years ago

Panning bounces after going back a page and scrolling

Categories

(Core :: Graphics: Layers, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27
blocking-b2g koi+
Tracking Status
firefox26 --- fixed
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: nhirata, Assigned: kats)

References

Details

(Keywords: regression)

Attachments

(2 files)

Gecko : 7dc9265ed90037fbd273a05d322637931191d87a
Gaia : 0035590eefc1ac6cfc05cfa868ed646f9f545e0c

1. launch browser
2. go to http://people.mozilla.com/~nhirata/html_tp/
3. select a web page
4. hit the back button
5. scroll

Expected: scrolling down
Actual: bouncing back to where you started from.
Need info for retesting
Flags: needinfo?(nhirata.bugzilla)
blocking-b2g: --- → koi?
Flags: needinfo?(nhirata.bugzilla)
Naoki, why did you nominate this? It sounds like expected behavior
Flags: needinfo?(nhirata.bugzilla)
Attached video bug909525.mov
Please see attached video.  the Actual result shows the web page bouncing up and down when trying to pan downwards to see the rest of the page.  Basically, you can't scroll down.
Flags: needinfo?(nhirata.bugzilla)
Sounds like an APZC regression.
Component: Gaia::Browser → Graphics: Layers
Keywords: regression
Product: Boot2Gecko → Core
Version: unspecified → Trunk
I recall filing this bug as soon as the APZ stuff landed.  I made my own build that day.
QA Contact: sarsenyev
Regression Window!

The issue doesn't reproduce on Buri:
Build ID: 20130730030200
Gecko: http://hg.mozilla.org/mozilla-central/rev/3d40d270c031
Gaia: ba5ff211fbf6a930326cc6a0d4a1205a7528630b
Platform Version: 25.0a1

The issue reproduces on Buri:
Build ID: 20130731030205
Gecko: http://hg.mozilla.org/mozilla-central/rev/c2b375f3a909
Gaia: 9bfceaa90e8b92a379432b67121afa3cd3f14c90
Platform Version: 25.0a1 

With some builds able to scroll down but not able to scroll up
Assignee: nobody → bugmail.mozilla
Attached patch PatchSplinter Review
Attachment #806369 - Flags: review?(tnikkel)
Attachment #806369 - Flags: review?(botond)
I suspect that backing out part 2 of bug 895905 might also fix this. That might be a cleaner way to do this; I will verify if that actually fixes this or not.
Yeah backing out part 2 of bug 895905 also fixes this. I would lean towards doing, what do you guys think?
Blocks: 895905
Do we still want to stop treating the root id as special? Can we? If so, do we have a plan for that?

If that is still the goal then I'm less concerned about how we hack around it in the mean time.
Yeah I think long term we would still like to stop treating it as special. Bug 900092 is on file for getting rid of it entirely if we can.
(But I don't have a concrete plan for accomplishing that)
Comment on attachment 806369 [details] [diff] [review]
Patch

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

::: dom/ipc/TabChild.cpp
@@ +356,5 @@
>  
> +    // Note that we cannot use FindScrollableFrameFor(ROOT_SCROLL_ID) because
> +    // it might return the root element from a different page in the case where
> +    // that page is in the bfcache and this page is not run through layout
> +    // before being drawn to the screen. Hence the code blocks below treat

Why don't non-ROOT_SCROLL_ID id's have this problem?
ROOT_SCROLL_ID is the only ScrollId that's reused across documents. All the other ones are unique because of the incrementing counter.
blocking-b2g: koi? → koi+
Comment on attachment 806369 [details] [diff] [review]
Patch

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

Looks good.

I agree that the proper solution is to eventually remove special handling of ROOT_SCROLL_ID, and I think this patch is more in line with that than reverting part 2 of bug 895905 (in particular, I think we'll want to keep the call to nsLayoutUtils::FindOrCreateIDFor() in nsDisplayList::PaintForFrame(), and reverting part 2 of bug 895905 would remove that). That said, I have no strong feelings about it.
Attachment #806369 - Flags: review?(botond) → review+
Attachment #806369 - Flags: review?(tnikkel) → review+
Comment on attachment 806369 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): combination of previous bugs landed in 26/b2g-1.2.
User impact if declined: on some pages, navigating away and going back breaks scrolling
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): i would say low risk but this code has proven to be quite brittle in the past. more testing on this would help mitigate risk
String or IDL/UUID changes made by this patch: none
Attachment #806369 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3b11b5385f84
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 806369 [details] [diff] [review]
Patch

koi+ blockers have automatic approval
Attachment #806369 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: