Closed Bug 742115 Opened 8 years ago Closed 8 years ago

Add config preferences to tweak panning speed

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 14
Tracking Status
firefox14 --- fixed
firefox15 --- verified
blocking-fennec1.0 --- beta+

People

(Reporter: BenWa, Assigned: kats)

References

Details

Attachments

(1 file, 2 obsolete files)

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
This is easy enough to implement by twiddling various constants in Axis.java. We'll probably want UX to weigh in on this though.
blocking-fennec1.0: --- → ?
Are these new dynamics because of rewrite? The feel of it seemed fine to me before. Benoit - can you give me a quick demo?
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.
Benoit, can you make the case to Madhava so we can make a decision here?
Assignee: nobody → bgirard
blocking-fennec1.0: ? → beta+
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?
Assignee: bgirard → nobody
re-noming for an assignee/re-evaluation.
blocking-fennec1.0: beta+ → ?
Assignee: nobody → joe
Summary: Fennec pans too fast → Add config preferences to tweak panning speed
Attached patch enpreferenceize (obsolete) — Splinter Review
This has only been very lightly tested, and I will not be around to commit it, so help is appreciated!
Attachment #614192 - Flags: review?(bugmail.mozilla)
Whiteboard: [autoland-try]
Whiteboard: [autoland-try] → [autoland-in-queue]
Unfortunately it seems that these prefs are not live, so we have to restart :(
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.
Whiteboard: [autoland-in-queue]
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.
Attachment #614192 - Attachment is obsolete: true
Attachment #614192 - Flags: review?(bugmail.mozilla)
Attachment #614390 - Flags: review?(chrislord.net)
Assignee: joe → bugmail.mozilla
blocking-fennec1.0: ? → beta+
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.
Attachment #614390 - Flags: review?(chrislord.net) → review+
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.
Attachment #614390 - Attachment is obsolete: true
Attachment #614774 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/84351dbcd62d
OS: Mac OS X → Android
Hardware: x86 → All
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/84351dbcd62d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.