605215 - [Regression] Multitouch: zoom goes toward the opposite side of the page with tab sidebar visible

Status: VERIFIED FIXED

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

Description

[Aakash Desai [:aakashd]]

Build Id:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101018 Namoroka/4.0b8pre Fennec/4.0b2pre

Steps to Reproduce:
1. Go to a zoomable page (I used AMO)
2. Wipe to the right to open the tab sidebar
3. Try to start a multitouch zoom on the "Categories" column

Actual Results:
The page zooms in from the right (opposite side of the page)
Expected Results:
The page zooms in to the left where my fingers are.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: patch

We used to always hideSidebar and hideTitlebar at pinch start, so that chrome client coordinates and browser coordinates had the same origin.

I'm not sure when we stopped doing hiding the toolbars during pinch.  We could just go back to hiding them, but I think it's better to make it work correctly while they are open.  This patch makes all coordinates relative to the browser element.

This only affects pinch zoom - we still hide toolbars and sidebars at the start of double-tap and volume-rocker zoom.  (With this patch we could stop doing that, but for double-tap in particular it feels better to automatically hide the sidebars so that the tapped element can fill the screen.)

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Uploaded the wrong version of the patch by accident.

Comment 3

[Matt Brubeck (:mbrubeck)]

Comment on attachment 484176 [details] [diff] [review]
patch v2

I've found a problem with this patch - if the urlbar is onscreen when you start zooming, it remains onscreen afterward even if the top of the page is no longer visible, and stays floated unless you drag to scroll down the page.

It might be best to continue to hide the toolbars either before or after zooming - or at least try to unfloat the urlbar.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Go back to hiding the sidebar and top bar after the zoom is finished, to fix the problem noted above.

Comment 5

[Benjamin Stover (:stechz)]

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

Oops accidentally hit r+ before asking where we actually hide the urlbar and sidebars?

Comment 6

[Matt Brubeck (:mbrubeck)]

(In reply to comment #5)
> Comment on attachment 484335 [details] [diff] [review]
> patch
> 
> Oops accidentally hit r+ before asking where we actually hide the urlbar and
> sidebars?

In Browser.setVisibleRect.

Comment 7

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/985f62cca3d8

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on build:

Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101019 Namoroka/4.0b8pre Fennec/4.0b2pre