center of zoom is offset when trying to zoom into things on the corner of the visible area

VERIFIED FIXED

Status

Firefox for Android Graveyard
Panning/Zooming
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: aakashd, Assigned: mbrubeck)

Tracking

Trunk
ARM
Android
Bug Flags:
in-litmus +

Details

Attachments

(1 attachment)

(Reporter)

Description

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

Steps to Reproduce:
1. Go to www.reddit.com
2. Find a small image on the page and begin to zoom into it

Actual Results:
The image is offset to the top right or bottom leftduring the zoom even though I started a zoom with it in the center.

Expected Results:
I'd expect the image to be in the center of my zoomed-in area.
(Reporter)

Updated

8 years ago
tracking-fennec: --- → ?
Flags: in-litmus?(mozaakash)
I don't understand the problem. Seems to work fine for me on reddit. The images on the left side of the page seem to zoom into the center of the screen.

If the problem still exists, you'll need to do better to explain it. Screenshots of before and after?
(Reporter)

Comment 2

8 years ago
The problem might be off, but Ben saw the issue when he offered it as a test run a week or two back. 

Can you offer better details than the stuff listed in the report, Ben?
This is about pinch zooming by the way.

It's definitely tough to explain and subtle. Sometimes when pinch zooming into anything that is not close to the center of the screen, the part of the page you "framed" with your fingers well step out of the frame the further you zoom in.

In order to fix this issue, I think we'll need something better than MozMagnifyGesture and its finger delta (perhaps use raw touch input). We need to apply a transform such that the place your fingers touch stay in the same position in content document space.
(In reply to comment #3)

> In order to fix this issue, I think we'll need something better than
> MozMagnifyGesture and its finger delta (perhaps use raw touch input). We need
> to apply a transform such that the place your fingers touch stay in the same
> position in content document space.

Why not fix it in MozMagnifyGesture? I don't like hacky FE fixes around platform code. I'm still not sure I believe this is even a problem. Until we are sure, it doesn't block.
tracking-fennec: ? → 2.0-
(Assignee)

Comment 5

8 years ago
We could extend MozMagnifyGesture to include the distance between the finger in pixels, in addition to the delta.  But this would be a touchscreen-specific extension, since it doesn't make sense for multitouch trackpads, or for hypothetical zoom gestures other than two-finger pinching.

I think the specific problem in this bug can be fixed without changing MozMagnifyGesture, however.  The zoomrect calculation is off slightly when panning and zooming at the same time; it should be possible to fix it so that the same page location always stays at the center of the gesture.
(Assignee)

Updated

8 years ago
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
(Assignee)

Comment 6

8 years ago
Created attachment 483471 [details] [diff] [review]
patch

This patch fixes the issue and also simplifies the zoom code slightly.  Instead of calculating the zoom rect based on the previous step, it always calculates it in relation to the starting rect.
Attachment #483471 - Flags: review?(mark.finkle)
Comment on attachment 483471 [details] [diff] [review]
patch

Code and premise seem fine to me, but let's get Ben's review too.
Attachment #483471 - Flags: review?(mark.finkle)
Attachment #483471 - Flags: review?(ben)
Attachment #483471 - Flags: review+
Comment on attachment 483471 [details] [diff] [review]
patch

>-    this._pinchZoomRect = AnimatedZoom.getStartRect()
>+    this._pinchStartRect = AnimatedZoom.getStartRect()

Let's get a semicolon while we're here.

Everything else looks great. Good job Matt :)
Attachment #483471 - Flags: review?(ben) → review+
(Assignee)

Comment 9

8 years ago
> Let's get a semicolon while we're here.

Done.

Pushed: http://hg.mozilla.org/mobile-browser/rev/3ef16e012815
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

8 years ago
woot, verified FIXED on build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101109 Namoroka/4.0b8pre Fennec/4.0b3pre
Status: RESOLVED → VERIFIED
(Reporter)

Comment 11

8 years ago
litmus testcases added https://litmus.mozilla.org/show_test.cgi?id=13789 to regression test this bug.
Flags: in-litmus?(mozaakash) → in-litmus+
You need to log in before you can comment on or make changes to this bug.