Last Comment Bug 721484 - Contenteditable demo doesn't work with touch events enabled
: Contenteditable demo doesn't work with touch events enabled
Status: RESOLVED FIXED
: testcase
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: P1 normal (vote)
: Firefox 12
Assigned To: Wesley Johnston (:wesj)
:
Mentors:
https://github.com/remy/html5demos/bl...
: 721212 721402 721712 (view as bug list)
Depends on: 723480 723772
Blocks: 603008
  Show dependency treegraph
 
Reported: 2012-01-26 12:22 PST by Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
Modified: 2012-02-07 07:57 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
testcase (1.63 KB, text/html)
2012-01-26 12:22 PST, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
Patch (656 bytes, patch)
2012-01-30 15:17 PST, Wesley Johnston (:wesj)
blassey.bugs: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2012-01-26 12:22:57 PST
Created attachment 591893 [details]
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) 2012-01-26 13:48:04 PST
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 2 Wesley Johnston (:wesj) 2012-01-30 14:36:54 PST
*** Bug 721402 has been marked as a duplicate of this bug. ***
Comment 3 Wesley Johnston (:wesj) 2012-01-30 14:38:04 PST
*** Bug 721712 has been marked as a duplicate of this bug. ***
Comment 4 Wesley Johnston (:wesj) 2012-01-30 14:49:05 PST
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) 2012-01-30 15:17:12 PST
Created attachment 592886 [details] [diff] [review]
Patch
Comment 7 Ed Morley [:emorley] 2012-01-31 06:49:13 PST
https://hg.mozilla.org/mozilla-central/rev/2fa163bb05d7
Comment 8 Wesley Johnston (:wesj) 2012-01-31 09:58:34 PST
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?) 2012-02-02 14:23:31 PST
Wes, request approval please
Comment 10 Wesley Johnston (:wesj) 2012-02-02 14:30:54 PST
*** Bug 721212 has been marked as a duplicate of this bug. ***
Comment 11 Wesley Johnston (:wesj) 2012-02-02 14:35:26 PST
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) 2012-02-02 16:03:12 PST
Comment on attachment 592886 [details] [diff] [review]
Patch

This is already on aurora
Comment 13 Matt Brubeck (:mbrubeck) 2012-02-02 16:58:07 PST
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) 2012-02-02 17:12:48 PST
(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?) 2012-02-02 22:44:34 PST
(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?) 2012-02-02 22:47:56 PST
(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) 2012-02-03 08:34:47 PST
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) 2012-02-03 10:50:58 PST
(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] 2012-02-05 13:42:42 PST
Comment on attachment 592886 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Beta 11.
Comment 20 Wesley Johnston (:wesj) 2012-02-07 07:57:25 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/c05f11ccd1b3

Note You need to log in before you can comment on or make changes to this bug.