Closed Bug 522504 Opened 13 years ago Closed 13 years ago

Panning speed is sometimes erratic

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
fennec1.0b5

People

(Reporter: stechz, Assigned: stechz)

References

Details

Attachments

(1 file, 1 obsolete file)

I think it is caused by how we determine our kinetic speed:

Fast mouse movements have a way of being drowned out by later mousemoves, even if they all occurred in quick succession.  If we get less resolution in our mousemove events, these big values aren't dampened out and speed is much faster.  Instead of basing final speed on position in queue, we should weight them by how close they occurred to mouseup for more predictable panning.
No longer blocks: 504390
Blocks: 504390
Attached patch Patch (obsolete) — Splinter Review
Although it isn't quite perfect, I don't have near as many crazy pans as I used to.  Please try on the N900 and see what you think.
Assignee: nobody → webapps
Attachment #406567 - Flags: review?(combee)
Comment on attachment 406567 [details] [diff] [review]
Patch

Not for nothing, this might be the sixth "adjust" to kinetic scrolling. I think everyone on the team has taken a crack at it. Since Doug was the last person to have a turn, I think he should take a look too.
Attachment #406567 - Flags: review?(doug.turner)
Comment on attachment 406567 [details] [diff] [review]
Patch

* Is event.timeStamp sane on WinMo?

* Comment for doDragStop mentions a secret third parameter, which is now the fourth parameter.
So far, code looks fine, but I can't R+ until I finish my build work to move things to /opt and can put something on my N900 again to test it out.
Comment on attachment 406567 [details] [diff] [review]
Patch

please make prefs for the new tweakables that you moved to the top of the file.

with that and mfinkle's nits, r=me.
Attachment #406567 - Flags: review?(doug.turner) → review+
Blocks: 511997
Comment on attachment 406567 [details] [diff] [review]
Patch

Sidebars aren't snapping correctly with this applied on desktop... at end of drag, they were staying partially on screen.
Attachment #406567 - Flags: review?(combee) → review-
Problem on Win32 desktop is that multiple events have same timestamp, so we get a timeDiff value of zero which means divide by zero in figuring out initial speed.
Problem is that code in addData that eliminates events with same timestamp wasn't preventing the push at end due to no early return.  Adding an early return helps.
Addresses preferences and bcombee's bug.
Attachment #406567 - Attachment is obsolete: true
Attachment #407098 - Flags: review?(combee)
Comment on attachment 407098 [details] [diff] [review]
Code review changes

Looks good on desktop browser, my issue was addressed.
Attachment #407098 - Flags: review?(combee) → review+
pushed:
https://hg.mozilla.org/mobile-browser/rev/6c3a8e6aeda4
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → B5
I think there's still some kinks with the zoom that I'd like to bring up in separate bugs, but this definitely helps with jitter on trying to click a link, having multiple pan events in quick successive actions among other panning actions in build:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091020
Fennec/1.0b5pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091020
Fennec/1.0b5pre

and 

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091020 Fennec/1.0a4pre
Status: RESOLVED → VERIFIED
Component: General → Panning/Zooming
You need to log in before you can comment on or make changes to this bug.