721484 - Contenteditable demo doesn't work with touch events enabled

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Martijn Wargers (dead)]

Attachment: testcase

When tapping on the contenteditable div, the vkb should come up and you should be able to type something in there.
This works fine in current trunk build, but not with dom.w3c_touch_events.enabled set to true in about:config.
Then the demo suddenly doesn't work.

I've investigated this with this testcase, which sort of does what that demo does:
http://people.mozilla.org/~mwargers/tests/contenteditable/contenteditable_focus_designmode.htm

With touch events disabled I get:
mouseover on DIV
mousemove on DIV
mousedown on DIV
focus on contenteditable element, designMode turned on
mouseup on DIV
click on DIV

With touch events enabled, I get:
touchstart on DIV
mousedown on DIV
focus on contenteditable element, designMode turned on
mouseover on DIV
mousemove on DIV
touchend on DIV
mouseup on DIV
click on DIV
mousemove on DIV
mousedown on DIV
blur on contenteditable element, designMode turned off
mouseup on DIV
click on DIV

Comment 1

[Wesley Johnston (:wesj)]

Hmm... looks like we're firing mouse events when we shouldn't. I would expect that to read more like:

touchstart on DIV
touchend on DIV
mousemove on DIV
mousedown on DIV
focus on contenteditable element, designMode turned on
mouseup on DIV
click on DIV

Comment 4

[Wesley Johnston (:wesj)]

Adding tracking flag. If we want touch events in 11, we'll need this.

This is caused by my own stupidity in adding MOZ_ONLY_TOUCH_EVENT=1 in mobile/android/confvars.sh and assuming that it would be defined when we compiled widget/android/nsWindow.cpp. Simple solution is adding it to the widget/android/Makefile.in like we do with the compositor here:

http://mxr.mozilla.org/mozilla-central/source/widget/android/Makefile.in#54

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 6

[Wesley Johnston (:wesj)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2fa163bb05d7

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2fa163bb05d7

Comment 8

[Wesley Johnston (:wesj)]

Not quite fixed yet :) Last part pushed to inbound now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/69ca6e6c57ea

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Wes, request approval please

Comment 11

[Wesley Johnston (:wesj)]

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

Regression caused by (bug #): 603008 multitouch
User impact if declined: Errors with multitouch events firing incorrectly
Testing completed (on m-c, etc.): landed on mc 1/2/2012
Risk to taking this patch (and alternatives if risky): Low risk. Should only affect builds with touch events enabled (i.e. mobile and b2g.

Comment 12

[Wesley Johnston (:wesj)]

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

This is already on aurora

Comment 13

[Matt Brubeck (:mbrubeck)]

This caused a regression in XUL fennec; there is a fix in bug 723746.  Please wait until both are approved before landing them in beta.

Comment 14

[Matt Brubeck (:mbrubeck)]

(In reply to Wesley Johnston (:wesj) from comment #8)
> Not quite fixed yet :) Last part pushed to inbound now.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/69ca6e6c57ea

This followup did not make it to Aurora.  (Also, it seems to have the wrong bug number in the commit message, and was not attached as a patch to this bug or to the one that it lists.)

Comment 15

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

(In reply to Matt Brubeck (:mbrubeck) from comment #13)
> This caused a regression in XUL fennec; there is a fix in bug 723746. 
> Please wait until both are approved before landing them in beta.

Matt - Is this the right bug for your comment? Bug 723746 has the UA regression fix.

Comment 16

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

(In reply to Matt Brubeck (:mbrubeck) from comment #14)
> (In reply to Wesley Johnston (:wesj) from comment #8)
> > Not quite fixed yet :) Last part pushed to inbound now.
> > 
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/69ca6e6c57ea
> 
> This followup did not make it to Aurora.  (Also, it seems to have the wrong
> bug number in the commit message, and was not attached as a patch to this
> bug or to the one that it lists.)

Wes - I saw this patch on bug 721079. Is that the right bug? Or should it be landed with this bug?

Comment 17

[Wesley Johnston (:wesj)]

Whoops. Yeah. I think just got really confused about what bug that patch went with. 

The patch is on bug 721079 and it has landed on central and is nom'd for Aurora (It's also in the Aurora and beta patch queue). I added a comment there with the hg link.

Comment 18

[Matt Brubeck (:mbrubeck)]

(In reply to Mark Finkle (:mfinkle) from comment #15)
> (In reply to Matt Brubeck (:mbrubeck) from comment #13)
> > This caused a regression in XUL fennec; there is a fix in bug 723746. 
> > Please wait until both are approved before landing them in beta.
> 
> Matt - Is this the right bug for your comment? Bug 723746 has the UA
> regression fix.

Oops.  This is the right bug, but I pasted the wrong regression.  This patch triggered a different XUL fennec regression, bug 723480.

Comment 19

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Mobile only - approved for Beta 11.

Comment 20

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c05f11ccd1b3