Closed Bug 712386 Opened 9 years ago Closed 9 years ago
Flinging after zooming allows unbounded zooming
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.
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+
(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.
9 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]
Comment on attachment 583907 [details] [diff] [review] Have a hard limit on zoom (v2) Investigating oranges
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]
Whiteboard: [leave open after inbound merge], [inbound]
9 years ago
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+
Landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/2f0e91546c19
Verified fixed on Firefox 11.0a2 (2012-01-10) 12.0a1 (2012-01-10) Device: Samsung Galaxy SII (Android 2.3)
Fixing flags I messed up when I landed this to aurora.
You need to log in before you can comment on or make changes to this bug.