747792 - Preventdefault doesn't work for touch events from inside iframe

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

See url, inside the black bordered box inside the iframes, just make some swipe movements.

Notice how the page moves inside Fennec. That should not be happening.

Also pinch zoom gestures shouldn't have any effect other than that black lines are drawn inside the black bordered box.

Tested on the Samsung Galaxy Nexus.

This works at least in a 2012-03-15 trunk build.

Also, notice how drawing lines in the top iframe are not bound by the iframe box, which it should. But this is a different bug. I filed bug 747791 for that.

Comment 1

[Martijn Wargers (dead)]

Video here: http://www.youtube.com/watch?v=qzZivRrCo6s

Comment 2

[Cristian Nicolae (:xti)]

Last good build: 03/09

Bad build: 03/10

Possible regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9ca66ce2672f&tochange=7702bca6b64d

--
Device: HTC Desire Z
OS: Android 2.3.3

Comment 3

[Martijn Wargers (dead)]

Regression from bug 742019?

Comment 4

[Tony Chung [:tchung]]

soft blocker?

Comment 5

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

Kats - If you think this shouldn't block, just renom and comment

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

Easy fix

Comment 7

[Wesley Johnston (:wesj)]

To explain the problem, we weren't notifing java of the listeners, so it allowed panning.

With our new touch handler, once we've started giving events to the touch handler we don't give the page another chance to stop prevent panning. We did that on purpose because trying to start and stop panning after the page responds leads to confusing situations for the user (and our backend)

Comment 8

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

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

I can r+ this, I suspected it was something like this but hadn't started looking for the relevant line of code yet :)

Comment 9

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d7e560c39dc3

Comment 10

[Wesley Johnston (:wesj)]

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

[Approval Request Comment]
Regression caused by (bug #): bug 742019 made it visible. always present
User impact if declined: iframes will not be able to prevent panning.
Testing completed (on m-c, etc.): landed on mc March 4
Risk to taking this patch (and alternatives if risky): Very low risk. Samll change for correctness, and I haven't seen this on real sites ever.
String changes made by this patch: None.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d7e560c39dc3

Comment 12

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Very low risk and blocking FN1.0. Approved for Aurora 14.

Comment 13

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/51e0f0461cbc

Comment 14

[Martijn Wargers (dead)]

I verified this for trunk.