771099 - Unable double tap zoom on maps.google.com

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Adrian Tamas (:AdrianT)]

Firefox Mobile Native 14 beta 11/ Nightly 16.0a1 2012-07-03
Device: HTC Desire (Android 2.2)

Steps to reproduce:
1. Go to maps.google.com.
2. Pinch to zoom or do a double tap to zoom.

Expected results:
The user can use gestures to zoom.

Actual results:
Nothing happens. The only way to zoom is to use the +/- icons.

Comment 1

[Wesley Johnston (:wesj)]

Pinch zoom is an evangelism thing (last I checked it worked fine with phony, please file separately), but I think double tap zoom (and clicking in general on maps) has regressed somewhere.

Comment 2

[Alex Keybl [:akeybl]]

Unless this is a new regression, I don't see this blocking release of 14.0.1. Now that mobile triage has ended, please make use of the tracking nomination flags instead of the blocking flag for all new critical issues.

Comment 3

[Wesley Johnston (:wesj)]

This is a regression. We fixed this in bug 719240, but removed the fix (accidently) in bug 742019. Fix coming.

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch

To reiterate, Google maps "tap" detection is stupid. They grab every touch event and re-dispatch it as a mouse one, which their mouse event handlers then pick up and drag the map with. If they don't get a touchmove they also fire a click event (or dblclick in some cases).

This rules out an app like, say a piano using force to detect a lot of things since the user would actually have to move their finger to get any force beyond the one seen on the initial touchstart. But maps is so prevalent on the web, I don't think we have much choice. I'm tempted to make this a maps.google.com only hack, but I don't think we need the extra code.

Comment 5

[Wesley Johnston (:wesj)]

I'd like to make sure this gets triaged (and pushed forward if we can, once it lands). The fix is essentially the same fix we had before, except moved because that code no longer exists.

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Wesley Johnston (:wesj) from comment #3)
> This is a regression. We fixed this in bug 719240, but removed the fix
> (accidently) in bug 742019. Fix coming.

But not a regression from FF14.0 (please email if I'm wrong). Tracking for 15 and 16.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 640446 [details] [diff] [review]
Patch

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

Not my favourite way to solve this problem (I would prefer throwing this over the wall to evangelism) but ok. r+ with comments addressed.

::: mobile/android/base/GeckoApp.java
@@ +3097,5 @@
>          LayerController layerController = getLayerController();
>          layerController.setLayerClient(mLayerClient);
>  
>          layerController.getView().getTouchEventHandler().setOnTouchListener(new View.OnTouchListener() {
> +            PointF initialPoint = null;

Make this "private PointF mInitialPoint" since it's a class variable.

@@ +3116,5 @@
> +                    if (PointUtils.subtract(point, initialPoint).length() > PanZoomController.PAN_THRESHOLD) {
> +                        initialPoint = null;
> +                    } else {
> +                        return true;
> +                    }

Just for readability I'd prefer this check to be inverted. So:

if (...) {
    if (PointUtils(...) < PanZoomController.PAN_THRESHOLD) {
        // comment about why we are doing this (for google maps, see bug#)
        return true;
    }

    initialPoint = null;
}

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/25ffee656138

Comment 9

[Wesley Johnston (:wesj)]

Adjusting this summary. Pinch-zoom should go to evang.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25ffee656138

Comment 11

[Adrian Tamas (:AdrianT)]

Created Bug 772794 to cover the pinch-to-zoom evangelism issue.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage comment]
This looks like a viable candidate for beta (15). Please nominate if it can be uplifted.

Comment 13

[Wesley Johnston (:wesj)]

Comment on attachment 640446 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 742019
User impact if declined: Google maps allows panning, but thats it.
Testing completed (on m-c, etc.): On mc for a week. No fall out I know of.
Risk to taking this patch (and alternatives if risky): Low risk. Mobile only change. Only affects pages using touch events
String or UUID changes made by this patch: None.

Comment 14

[Wesley Johnston (:wesj)]

Comment on attachment 640446 [details] [diff] [review]
Patch

Doh. This was already on Aurora... kinda. Beta request?

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 742019
User impact if declined: Google maps allows panning, but thats it.
Testing completed (on m-c, etc.): On mc for a week. No fall out I know of.
Risk to taking this patch (and alternatives if risky): Low risk. Mobile only change. Only affects pages using touch events
String or UUID changes made by this patch: None.

Comment 15

[Alex Keybl [:akeybl]]

Comment on attachment 640446 [details] [diff] [review]
Patch

Good to go for Beta 15.

Comment 16

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-beta/rev/107ecf663341