941592 - Defect - double-tap to zoom fails if the user zooms out

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Pan and Zoom, defect, P2)

Description

[Jim Mathies [:jimm]]

str:

1) double tap to zoom in
2) manually zoom out
3) double tab to zoom in again

result:

no action

We should check browser scale and reset the zoomed flag here - 

http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/contenthandlers/Content.js#377

Comment 1

[Jim Mathies [:jimm]]

Attachment: fix

Comment 2

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8336009 [details] [diff] [review]
fix

There's no need to store _isZoomedIn as a flag if we're just going to overwrite it based on the zoom level each time we read it.  We could just check the zoom level directly.

Just checking if (resolution > 1) is not ideal either -- it creates an opposite problem, where double-tap to zoom *in* is broken if you're already zoomed in a little.  Really we should check whether zooming to the tapped element would change the zoom level significantly from its current value.  If so, zoom to the element; otherwise zoom out.

Comment 3

[Matt Brubeck (:mbrubeck)]

(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> Just checking if (resolution > 1) is not ideal either -- it creates an
> opposite problem, where double-tap to zoom *in* is broken if you're already
> zoomed in a little.  Really we should check whether zooming to the tapped
> element would change the zoom level significantly from its current value. 
> If so, zoom to the element; otherwise zoom out.

Alternately we could make this code listen for "manual" (pinch/keyboard/wheel) zoom events, and set _isZoomedIn to false when one of those happens.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

This is the start of a patch that shamelessly copies a bunch of code from TabChild/BrowserElementPanning.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: patch

I realized we can do this without schlepping a ton of data to the content script on every update (as in my previous patch).  Instead we do the calculation in the chrome script where we already have this data.

This also fixes a bug where browser.contentViewportBounds was returning all NaN because it was passing an nsIDOMClientRect instead of a Rect object, and a bug in ptClientToBrowser and rectClientToBrowser where scroll coordinates were scaled incorrectly, and cleans up some code in apzc.js so it no longer adds and removes listeners for every tab.

Comment 6

[Jim Mathies [:jimm]]

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

Review of attachment 8341783 [details] [diff] [review]:
-----------------------------------------------------------------

This fixes up a selection bounds bug I spotted last weekend as well. Thanks!

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/fx-team/rev/7b8cc2b3568b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7b8cc2b3568b