742115 - Add config preferences to tweak panning speed

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Benoit Girard (:BenWa)]

Currently we pan too fast on fennec, a small fling is sufficient to scroll from the top to the bottom of the page. I feel like the user experience of this is terrible with people with poor motor skills or even myself.

The current speed is also too high for our rendering engine to keep up even on desktop hardware.

I suggest that we reasonably reduce the panning speed. Here are a few things we can try:
1) Reduce the acceleration from a single fling
2) Add a max velocity
3) Adjust friction

Comment 1

[Kartikaya Gupta (email:[e-mail])]

This is easy enough to implement by twiddling various constants in Axis.java. We'll probably want UX to weigh in on this though.

Comment 2

[Madhava Enros [:madhava]]

Are these new dynamics because of rewrite? The feel of it seemed fine to me before. Benoit - can you give me a quick demo?

Comment 3

[Benoit Girard (:BenWa)]

They changed from xul to native but haven't changed since the OMTC/Maple feature landed. I can show you if you drop by at my desk.

Comment 4

[Joe Drew (not getting mail)]

Benoit, can you make the case to Madhava so we can make a decision here?

Comment 5

[Benoit Girard (:BenWa)]

I discussed with madhava. There's no a huge difference but there seems to be some room we can play around with to reduce thing. I think the first thing we should do it decrease the speed at which we pan and increase the friction to keep the fling distance traveled the same.

We need to find someone to own this task. The code leave in Axis.java and is trivial to implement on a technical level, it just needs careful tweaking of the factors.

We should produce some tests build with parameters we think are improvements. Any takers?

Comment 6

[Erin Lancaster [:elan]]

re-noming for an assignee/re-evaluation.

Comment 7

[Joe Drew (not getting mail)]

Attachment: enpreferenceize

This has only been very lightly tested, and I will not be around to commit it, so help is appreciated!

Comment 8

[Joe Drew (not getting mail)]

Unfortunately it seems that these prefs are not live, so we have to restart :(

Comment 9

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 614192
	Branch: mozilla-central => try
Patch 614192 could not be applied to mozilla-central.
patching file mobile/android/app/mobile.js
Hunk #1 FAILED at 703
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/app/mobile.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patchset could not be applied and pushed.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: enpreferenceize (rebased and modified)

I'm shamelessly stealing :joe's patch since he said he wouldn't be around for the next few days. This updated version applies on top of the new v3 patch in bug 744241 and uses the map-of-prefs introduced there to eliminate method explosion.

Comment 11

[Chris Lord [:cwiiis]]

Comment on attachment 614390 [details] [diff] [review]
enpreferenceize (rebased and modified)

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

r+ with these two comments addressed.

::: mobile/android/app/mobile.js
@@ +737,5 @@
> +// The fraction of the surface which can be overscrolled before it must snap back, in 1000ths.
> +pref("gfx.scrolling.overscroll_snap_limit", 300);
> +// The minimum amount of space that must be present for an axis to be considered scrollable,
> +// in 1/1000ths of pixels.
> +pref("gfx.scrolling.min_scrollable_distance", 500);

I don't get all this 1000th's business, but it's in xul-fennec too so I assume there's some reason for it...

One thing I will say, the prefix shouldn't be gfx.scrolling, it should be ui.scrolling (going on the prefix of PanZoomController.java)

::: mobile/android/base/ui/Axis.java
@@ +92,5 @@
> +        VELOCITY_THRESHOLD = getIntPref(prefs, PREF_SCROLLING_VELOCITY_THRESHOLD, 10);
> +        MAX_EVENT_ACCELERATION = getFloatPref(prefs, PREF_SCROLLING_MAX_EVENT_ACCELERATION, 12);
> +        OVERSCROLL_DECEL_RATE = getFloatPref(prefs, PREF_SCROLLING_OVERSCROLL_DECEL_RATE, 40);
> +        SNAP_LIMIT = getFloatPref(prefs, PREF_SCROLLING_OVERSCROLL_SNAP_LIMIT, 300);
> +        MIN_SCROLLABLE_DISTANCE = getFloatPref(prefs, PREF_SCROLLING_MIN_SCROLLABLE_DISTANCE, 500);

We shouldn't have all these values in two places, it's just going to get desynced at some point. Either they should only be here or only above.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: enpreferenceize (v3)

Updated patch, carrying r+

(In reply to Chris Lord [:cwiiis] from comment #11)
> I don't get all this 1000th's business, but it's in xul-fennec too so I
> assume there's some reason for it...

As discussed on IRC, presumably this because this was preferable to having string prefs and parsing floats from it, since we can't have float prefs.

> One thing I will say, the prefix shouldn't be gfx.scrolling, it should be
> ui.scrolling (going on the prefix of PanZoomController.java)

Fixed

> We shouldn't have all these values in two places, it's just going to get
> desynced at some point. Either they should only be here or only above.

Fixed. I now keep the default values in one place in Java only, and initialize the prefs to -1. When setting prefs any negative prefs are ignored in favour of the default value.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/84351dbcd62d

Comment 14

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/84351dbcd62d

Comment 15

[Cristian Nicolae (:xti)]

Panning speed is changed now on the latest Nightly. Long pages are no longer pannable with just one fast pan move. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-23)
Device: Galaxy Nexus
OS: Android 4.0.2