Closed Bug 999162 Opened 10 years ago Closed 10 years ago

Calling scrollTo(0, 0) while at the top of a page prevents subsequent panning

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: kgrandon, Assigned: kats)

References

Details

Attachments

(2 files)

Calling scrollTo(0,0) works if I scroll down the page a bit, but not if I do this while at the top of a page. Note: this only impacts the first pan action, subsequent pans do not suffer from this.
A bit confused.  If you're already at the top of the page, what do you expect scrollTo(0,0) to do?
It doesn't need to do anything, but the next pan action after calling it should continue to work.
Note that while we may be able to avoid calling it if at the top, we would have to check the scrollTop position which would introduce unnecessary performance cost/extra code. We need to fix it so that we can call scrollTo(0,0) while at the top and have panning working like normal afterward.
(In reply to Kevin Grandon :kgrandon from comment #2)
> It doesn't need to do anything, but the next pan action after calling it
> should continue to work.

Got it.  I changed the subject to clarify - I know I was confused.
Summary: Calling scrollTo(0, 0) while at the top of a page prevents panning → Calling scrollTo(0, 0) while at the top of a page prevents subsequent panning
I'll look into this.
Assignee: nobody → bugmail.mozilla
So it's not actually the scrollTo() call itself that's the problem, it's that clicking on the link also changes the URL because it's a fragment jump, so we get a NotifyLayersUpdated with "aIsFirstPaint" set to true on the next paint. That causes the scroll offset and state to get reset, which cancels out the scroll that the user is in the middle of.

If the "click" event is cancelled (i.e. e.preventDefault()) then this issue doesn't appear.
Attached patch PatchSplinter Review
This fixes the problem but may cause regressions. I'll need to test a few scenarios before I request review.
No-Jun, do you have time to take a look at testing this, with the patches Kats has? Just to accelerate some other work.
Flags: needinfo?(npark)
Attachment #8412025 - Flags: review?(drs+bugzilla)
Tested by flashing latest gecko with patch on 1.4 build.  now the scrolling is smooth when running the scrollTest app (i.e. not scrolling, click the top link, then scroll down).  

Before applying the patch, it showed jitter since it did not execute the scroll action right after calling scrollTo(0,0).
Flags: needinfo?(npark)
Comment on attachment 8412025 [details] [diff] [review]
Patch

LGTM, I'm actually not sure why it wasn't this way to begin with. I didn't test this since you and No-Jun both have.
Attachment #8412025 - Flags: review?(drs+bugzilla) → review+
Awesome, thanks guys!
https://hg.mozilla.org/mozilla-central/rev/60c2852b75ee
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.
You need to log in before you can comment on or make changes to this bug.