Closed
Bug 712386
Opened 12 years ago
Closed 12 years ago
Flinging after zooming allows unbounded zooming
Categories
(Firefox for Android Graveyard :: General, defect, P2)
Tracking
(firefox11 verified, firefox12 verified, fennec11+)
VERIFIED
FIXED
Firefox 12
People
(Reporter: kats, Assigned: kats)
Details
Attachments
(1 file, 1 obsolete file)
1.68 KB,
patch
|
kats
:
review+
christian
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
STR: 1. Open some page and wait for it to load 2. Put both fingers down on the screen and pinch zoom in (make content larger) 3. Continue doing step 2 until the zoom is greater than 4.0 (when this happens lifting both fingers will result in the zoom snapping back to 4.0) 4. Do another pinch zoom to drive the zoom above 4.0, then lift one finger and fling the page. Expected results: After the fling stops, or while the fling is happening, or at some point during this sequence of events, the page reduces the zoom level back to 4.0 Observed results: Page zoom remains above 4.0. Step 4 can be repeated over and over to drive the zoom as high as desired. A variation of this is to just keep one finger down at all times, and use the other finger to do multiple pinches, one after another. This also allows zooming in as much as desired, instead of clamping it to 4.0.
Updated•12 years ago
|
Assignee: nobody → bugmail.mozilla
Priority: -- → P2
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #583885 -
Flags: review?(chrislord.net)
Comment 2•12 years ago
|
||
Comment on attachment 583885 [details] [diff] [review] Have a hard limit on zoom Review of attachment 583885 [details] [diff] [review]: ----------------------------------------------------------------- I guess you meant to use ALLOWED_OVERZOOM in the calculation of the zoom past MAX_ZOOM. r+ with this rectified. ::: mobile/android/base/ui/PanZoomController.java @@ +102,5 @@ > private static final float MIN_SCROLLABLE_DISTANCE = 0.5f; > // The maximum amount we allow you to zoom into a page > private static final float MAX_ZOOM = 4.0f; > + // The amount we allow (temporarily) zooming up to over MAX_ZOOM. There will be resistance here. > + private static final float ALLOWED_OVERZOOM = 2.0f; This is never used. Either it should be removed, or it should be used in the calculations below. @@ +943,5 @@ > + // apply resistance when zooming past MAX_ZOOM, > + // such that it asymptotically reaches MAX_ZOOM + 1.0 > + // but never exceeds that > + float excessZoom = newZoomFactor - MAX_ZOOM; > + excessZoom = 1.0f - (float)Math.exp(-excessZoom); I guess this should actually reach MAX_ZOOM + ALLOWED_OVERZOOM and the excessZoom equation should be modified appropriately.
Attachment #583885 -
Flags: review?(chrislord.net) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #2) > I guess you meant to use ALLOWED_OVERZOOM in the calculation of the zoom > past MAX_ZOOM. r+ with this rectified. Good catch. I had initially set up ALLOWED_OVERZOOM and was scaling up excessZoom by that before adding it to MAX_ZOOM, but realized that the behaviour with that didn't feel quite right. The problem is that the spanRatio is multiplied on top of the last zoom factor (which already includes some resistance), so the function ends up being not what I expected. Long story short, it worked better without the ALLOWED_OVERZOOM so I just took that out. Uploading updated patch and carrying r+, will land when tree is open.
Attachment #583885 -
Attachment is obsolete: true
Attachment #583907 -
Flags: review+
Updated•12 years ago
|
Hardware: All → ARM
Assignee | ||
Comment 4•12 years ago
|
||
Landed on m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/d4f9960e286a
status-firefox11:
--- → affected
Whiteboard: [inbound], [leave open after inbound merge]
Target Milestone: --- → Firefox 12
Assignee | ||
Updated•12 years ago
|
Attachment #583907 -
Flags: approval-mozilla-aurora?
Comment 5•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b58836d3fa78 Backed out on suspicion of causing Android m1 and m2 orange.
Whiteboard: [inbound], [leave open after inbound merge] → [leave open after inbound merge]
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 583907 [details] [diff] [review] Have a hard limit on zoom (v2) Investigating oranges
Attachment #583907 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 7•12 years ago
|
||
Re-landed on m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/18eead5419ff Try run ran green here: https://tbpl.mozilla.org/?tree=Try&rev=d5f66b34017d Random oranges appear to be caused by the patch for bug 712761.
Whiteboard: [leave open after inbound merge] → [leave open after inbound merge], [inbound]
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/18eead5419ff
Whiteboard: [leave open after inbound merge], [inbound]
Assignee | ||
Updated•12 years ago
|
Attachment #583907 -
Flags: approval-mozilla-aurora?
Comment on attachment 583907 [details] [diff] [review] Have a hard limit on zoom (v2) [triage comment] Approved for aurora. Polish issue, mobile only. Please remember to land the patch as it landed on m-c (looks like there was an additional change needed for the r+).
Attachment #583907 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 10•12 years ago
|
||
Landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/2f0e91546c19
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 12 → Firefox 11
Updated•12 years ago
|
tracking-fennec: --- → 11+
Comment 11•12 years ago
|
||
Verified fixed on Firefox 11.0a2 (2012-01-10) 12.0a1 (2012-01-10) Device: Samsung Galaxy SII (Android 2.3)
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 12•12 years ago
|
||
Fixing flags I messed up when I landed this to aurora.
status-firefox12:
--- → verified
Target Milestone: Firefox 11 → Firefox 12
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
•