Closed
Bug 1116048
Opened 9 years ago
Closed 9 years ago
The 'browser.ui.zoom.force-user-scalable' pref is not toggleable
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox34 unaffected, firefox35 wontfix, firefox36 verified, firefox37 verified, fennec36+)
VERIFIED
FIXED
Firefox 37
Tracking | Status | |
---|---|---|
firefox34 | --- | unaffected |
firefox35 | --- | wontfix |
firefox36 | --- | verified |
firefox37 | --- | verified |
fennec | 36+ | --- |
People
(Reporter: kats, Assigned: kats)
Details
Attachments
(1 file, 1 obsolete file)
3.60 KB,
patch
|
bnicholson
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
STR: 1. Start Fennec, load about:config 2. Type "force-" in the search box. This should make the browser.ui.zoom.force-user-scalable pref appear at the top of the list 3. Tap on the pref to give it focus 4. Tap on it again to toggle it (or hit the toggle button if you prefer) ER: Pref toggles AR: Pref toggles, then immediately toggles back. i.e. there's no way to actually change the value of the pref. Regression in Fennec 35 from what I can tell using the builds I have installed on my device.
Assignee | ||
Updated•9 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 1•9 years ago
|
||
It looks like what's happening is the onSingleTapUp [1] fires with double tap disallowed (so waitForDoubleTap() returns false). This flips the pref and changes the zoom constraints so that now double tap is allowed. This means when onSingleTapConfirmed [2] runs waitForDoubleTap() returns true and another Gesture:SingleTap is dispatched. [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/JavaPanZoomController.java?rev=8081294c87ee#1362 [2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/JavaPanZoomController.java?rev=8081294c87ee#1375
Assignee | ||
Comment 2•9 years ago
|
||
Kind of hacky but I couldn't think of anything better.
Attachment #8541996 -
Flags: review?(bnicholson)
Comment 3•9 years ago
|
||
Comment on attachment 8541996 [details] [diff] [review] Patch Review of attachment 8541996 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense, but I think we can make this less hacky. How about the following: 1) Rename mMediumPress to mWaitForDoubleTap. 2) Set mWaitForDoubleTap = mTarget.getZoomConstraints().getAllowDoubleTapZoom() at [1]. 3) Set mWaitForDoubleTap to false at [2]. 4) Remove waitForDoubleTap(), and check the boolean directly instead (which won't change when the pref changes). Makes sense in my head, but feel free to reflag r? if this won't work for some reason. [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/JavaPanZoomController.java?rev=8081294c87ee#1336 [2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/JavaPanZoomController.java?rev=8081294c87ee#1348
Attachment #8541996 -
Flags: review?(bnicholson) → review-
Assignee | ||
Comment 4•9 years ago
|
||
That is much cleaner, thanks!
Assignee | ||
Comment 5•9 years ago
|
||
Assignee: nobody → bugmail.mozilla
Attachment #8541996 -
Attachment is obsolete: true
Attachment #8542173 -
Flags: review?(bnicholson)
Comment 6•9 years ago
|
||
Comment on attachment 8542173 [details] [diff] [review] Patch v2 Review of attachment 8542173 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/gfx/JavaPanZoomController.java @@ +122,5 @@ > /* The per-frame zoom delta for the currently-running AUTONAV animation. */ > private float mAutonavZoomDelta; > /* The user selected panning mode */ > private AxisLockMode mMode; > + /* Whether or to wait for a double-tap before dispatching a single-tap */ Nit: "whether or not" or just "whether".
Attachment #8542173 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Thanks. Fixed and landed: https://hg.mozilla.org/integration/fx-team/rev/da105bbc729c
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8542173 [details] [diff] [review] Patch v2 Approval Request Comment [Feature/regressing bug #]: unsure, something in Fx35 [User impact if declined]: unable to toggle the browser.ui.zoom.force-user-scalable pref from false to true using about:config in Fennec. (that pref specifically is the only one affected) [Describe test coverage new/current, TBPL]: tested locally [Risks and why]: should be pretty low risk; relatively small patch in fennec-only code [String/UUID change made/needed]: none Even though this is a regression in 35 it's pretty late in the beta cycle and I don't think it's worth uplifting. I don't think that many people flip this pref that it will be an issue. People who have already flipped the pref from false to true shouldn't be affected unless they flip it back to false and then try to flip it back to true again.
Attachment #8542173 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/da105bbc729c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•9 years ago
|
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8542173 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/a571d150a8f1
Comment 11•9 years ago
|
||
Tapping on the "toggle" button, the pref will change into true and will remain so. Tapping once again it will become false, so: Verified as fixed on: Device: Alcatel One Touch OS: Android 4.1.2 Build: Firefox for Android 37.0a1 (2015-01-05) and Firefox for Android 36.0a2 (2014-01-05)
Updated•9 years ago
|
tracking-fennec: ? → 36+
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•