If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Browser continues to scroll after swipe until finger lifted

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bkelly, Assigned: cwiiis)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
On a fresh master sync'd today (5/8/2013) I see strange scrolling behavior in the B2G browser:

 1) When I swipe up or down to scroll, the window keeps scrolling until I lift my finger.
 2) Sometimes the scrolling goes into a continuous jumping cycle when I swipe near the top or bottom of the page.

An example can be seen in the following video.  (Apologies for the poor quality and orientation.)

  https://www.dropbox.com/s/xa4m78f6k4mz983/20130508_b2g_master_scroll_issue.MOV

This is on a Buri device.

Gaia Git revision:  3f766bccdd9c0961b33f2085089c1f575143bd8b
This shouldn't be anything to do with your Gaia version, async pan and zoom is handled in Gecko. What gecko build do you have?
(Reporter)

Comment 2

4 years ago
Gecko was a fresh mozilla-central.

hg changeset:  131199:b980d32c366f

But I first saw this last week, so its been around for at least that long.
(Reporter)

Comment 3

4 years ago
I can't reproduce this on the emulator.  Not sure if that's expected or not given the weirdness of trying to swipe with a mouse.
(Reporter)

Comment 4

4 years ago
I also cannot reproduce on an unagi.  It looks like this may be specific to the hamachi platform.
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Summary: browser continues to scroll after swipe until finger lifted → [hamachi] browser continues to scroll after swipe until finger lifted
(Reporter)

Comment 5

4 years ago
Setting back to general category for now until I can figure out why component is the cause.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Component: Gaia::Browser → General
(Reporter)

Comment 6

4 years ago
I flashed my phone with the following combination of software:

  b2g-manifest: master
  b2g-gaia:     master
  gecko:        b2g18

And the unusual scroll behavior did not occur.  So it does appear to be a gecko issue.

It seems odd to me that it only appears on the buri device and not the unagi, however.
(Assignee)

Comment 7

4 years ago
I'm not sure what device the 'buri' is, but I see this on a Geeksphone Peak and a Geeksphone Keon building master of everything. Very likely an AsyncPanZoomController bug.
(Reporter)

Comment 8

4 years ago
I just flashed my buri with the new firmware from 5/8 and this problem persists.  Since the firmware had new touch drivers there was some hope it might resolve the issue.

Also, I've done some bisecting and the problem is present as far back as 127775:842c175f344c in mozilla-central.  Unfortunately, somewhere around 127753:051cf1c1449c my b2g process stops booting properly, so I've had issues going much further back.
(Assignee)

Comment 9

4 years ago
Created attachment 750922 [details] [diff] [review]
Fix continued movement when finger held on screen

This fixes the issue. I guess this may have been introduced to help a device with bad touch events, but the logic is faulty - this should be dealt with some other way.
Attachment #750922 - Flags: review?(ajones)
(Assignee)

Comment 10

4 years ago
I'm removing the 'hamachi' keyword, this isn't specific to any platform.
Summary: [hamachi] browser continues to scroll after swipe until finger lifted → Browser continues to scroll after swipe until finger lifted
(Reporter)

Comment 11

4 years ago
Thanks Chris!  As expected, attachment 750922 [details] [diff] [review] works for my Buri as well.

If you don't mind, I'll throw the assignment over to you.
Assignee: bkelly → chrislord.net
(Assignee)

Comment 12

4 years ago
Comment on attachment 750922 [details] [diff] [review]
Fix continued movement when finger held on screen

Handing review over to kats.

Further comment, I think the mPos != aPos check just needs to be gotten rid of altogether and newVelocity should be calculated over time rather than on a touch-move instance. (the second part of that can be dealt with in another bug)
Attachment #750922 - Flags: review?(ajones) → review?(bugmail.mozilla)
Comment on attachment 750922 [details] [diff] [review]
Fix continued movement when finger held on screen

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

This looks ok to me. Make sure you fix s/Asyn/Async/ in the commit message before landing.
Attachment #750922 - Flags: review?(bugmail.mozilla) → review+
Also CC'ing :tatiana and :drs since they added/reviewed this code in bug 824511, and might have objections.
Comment on attachment 750922 [details] [diff] [review]
Fix continued movement when finger held on screen


> void Axis::UpdateWithTouchAtDevicePoint(int32_t aPos, const TimeDuration& aTimeDelta) {
>-  if (mPos == aPos) {
>-    // Does not make sense to calculate velocity when distance is 0
>-    return;
>-  }

You should also prevent adding 0 velocity to the queue
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/Axis.cpp#124
Attachment #750922 - Flags: review-
> Further comment, I think the mPos != aPos check just needs to be gotten rid
> of altogether and newVelocity should be calculated over time rather than on
> a touch-move instance.
I was proposing same thing someday... but drs told me that we need to handle circle gesture kinetic motion... not sure why
(Assignee)

Comment 17

4 years ago
(In reply to Oleg Romashin (:romaxa) from comment #15)
> Comment on attachment 750922 [details] [diff] [review]
> Fix continued movement when finger held on screen
> 
> 
> > void Axis::UpdateWithTouchAtDevicePoint(int32_t aPos, const TimeDuration& aTimeDelta) {
> >-  if (mPos == aPos) {
> >-    // Does not make sense to calculate velocity when distance is 0
> >-    return;
> >-  }
> 
> You should also prevent adding 0 velocity to the queue
> http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/Axis.cpp#124

You shouldn't, otherwise the view will always move when your finger stops, even if your finger has been still for a long time.
(Assignee)

Comment 18

4 years ago
(In reply to Oleg Romashin (:romaxa) from comment #16)
> > Further comment, I think the mPos != aPos check just needs to be gotten rid
> > of altogether and newVelocity should be calculated over time rather than on
> > a touch-move instance.
> I was proposing same thing someday... but drs told me that we need to handle
> circle gesture kinetic motion... not sure why

I don't see how these things are mutually exclusive?
(Assignee)

Comment 19

4 years ago
Pushed to inbound with removal of mPos == aPos check confirmed on IRC:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2313616e4203

If we need to do something like this (which we probably do, we certainly do so in JavaPanZoomController), we need to do it in a way that doesn't break common use-cases (like wanting to stop scrolling).
https://hg.mozilla.org/mozilla-central/rev/2313616e4203
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.