The default bug view has changed. See this FAQ.

Contenteditable demo doesn't work with touch events enabled

RESOLVED FIXED in Firefox 11

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: wesj)

Tracking

({testcase})

Trunk
Firefox 12
ARM
Android
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed, fennec11+)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
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
Assignee: nobody → mbrubeck
Priority: -- → P1
(Assignee)

Updated

5 years ago
Duplicate of this bug: 721402
(Assignee)

Updated

5 years ago
Duplicate of this bug: 721712
(Assignee)

Comment 4

5 years ago
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
(Assignee)

Comment 5

5 years ago
Created attachment 592886 [details] [diff] [review]
Patch
Assignee: mbrubeck → wjohnston
Attachment #592886 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #592886 - Flags: review? → review?(blassey.bugs)
Attachment #592886 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 6

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/2fa163bb05d7
Whiteboard: [inbound]
(Assignee)

Updated

5 years ago
tracking-firefox11: --- → ?
tracking-fennec: --- → 11+
tracking-firefox11: ? → ---
https://hg.mozilla.org/mozilla-central/rev/2fa163bb05d7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
(Assignee)

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/69ca6e6c57ea
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

5 years ago
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Wes, request approval please
(Assignee)

Updated

5 years ago
Duplicate of this bug: 721212
(Assignee)

Comment 11

5 years ago
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.
Attachment #592886 - Flags: approval-mozilla-beta?
Attachment #592886 - Flags: approval-mozilla-aurora?
Depends on: 723480
(Assignee)

Comment 12

5 years ago
Comment on attachment 592886 [details] [diff] [review]
Patch

This is already on aurora
Attachment #592886 - Flags: approval-mozilla-aurora?
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.
status-firefox11: --- → affected
status-firefox12: --- → fixed
Whiteboard: [inbound]
Depends on: 723772
(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.)
status-firefox12: fixed → affected
(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.
(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?
(Assignee)

Comment 17

5 years ago
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.
(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.
status-firefox12: affected → fixed
Comment on attachment 592886 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Beta 11.
Attachment #592886 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 20

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/c05f11ccd1b3
status-firefox11: affected → fixed
You need to log in before you can comment on or make changes to this bug.