597210 - Multitouch: page jumps when zooming out to full page width on a mobile-friendly page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Aakash Desai [:aakashd]]

Build Id:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100916 Namoroka/4.0b7pre Fennec/2.0b1pre

Steps to Reproduce:
1. Go to http://blogs.babycenter.com/mom_stories/
2. Try to zoom out

Actual Results:
The page will jump to the right and half of the screen (left-side) will be gray for a split second during the zoom phase.

Expected Results:
The page shouldn't jump during the middle of zoom out.

Comment 1

[Matt Brubeck (:mbrubeck)]

This seems to happen whenever you start a pinch gesture while already zoomed in a ways *and* already panned away from the left edge of the page.

Comment 2

[Matt Brubeck (:mbrubeck)]

This should maybe block beta.  If you are zoomed in close, near the bottom right of a page, pinch zooming is totally broken - you jump to a completely different area of the page.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This should fix all jumps during panning.

Rather than an incremental patch, this completely replaces the zoomrect calculation.  Rather than use _getZoomRectForPoint, the new pinch zoom code always starts with the initial visible rect, and transforms it based on the gesture movements.

This also fixes a slightly-related bug in _getZoomRectForPoint where double-tapping near the right or bottom edge of the page would animate incorrectly.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

uploaded the wrong version

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 476257 [details] [diff] [review]
patch

>diff -r 920af3b32bd2 chrome/content/AnimatedZoom.js

>+  getStartRect: function getStartRect() {
>+    let browserRect = Rect.fromRect(getBrowser().getBoundingClientRect());
>+    let scroll = getBrowser().getPosition();
>+    return browserRect.translate(scroll.x, scroll.y);
>+  },

Local for getBrowser() ?

>diff -r 920af3b32bd2 chrome/content/InputHandler.js

Stover bitrotted you in this file for sure

>diff -r 920af3b32bd2 chrome/content/browser.js

>   _getZoomRectForPoint: function _getZoomRectForPoint(x, y, zoomLevel) {
>     x = x * getBrowser().scale;
>     y = y * getBrowser().scale;
> 
>     zoomLevel = Math.min(ZoomManager.MAX, zoomLevel);
>-    let zoomRatio = zoomLevel / getBrowser().scale;
>+    let oldScale = getBrowser().scale;
>+    let zoomRatio = zoomLevel / oldScale;
>     let newVisW = window.innerWidth / zoomRatio, newVisH = window.innerHeight / zoomRatio;
>     let result = new Rect(x - newVisW / 2, y - newVisH / 2, newVisW, newVisH);
> 
>     // Make sure rectangle doesn't poke out of viewport
>-    return result.translateInside(new Rect(0, 0, getBrowser().contentDocumentWidth, getBrowser().contentDocumentHeight));
>+    return result.translateInside(new Rect(0, 0, getBrowser().contentDocumentWidth * oldScale,
>+                                                 getBrowser().contentDocumentHeight * oldScale));

Local for getBrowser()

Comment 7

[Matt Brubeck (:mbrubeck)]

(In reply to comment #6)
> Local for getBrowser() ?

Done.

> >diff -r 920af3b32bd2 chrome/content/InputHandler.js
> Stover bitrotted you in this file for sure

Not if I bitrot him first!  >:)

> Local for getBrowser()

Done, and pushed:
http://hg.mozilla.org/mobile-browser/rev/c3b00aea2425

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100920 Namoroka/4.0b7pre Fennec/2.0b1pre