Last Comment Bug 648573 - Implement touch event interfaces
: Implement touch event interfaces
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
:
:
Mentors:
Depends on: 652814
Blocks: 544614 651984
  Show dependency treegraph
 
Reported: 2011-04-08 10:32 PDT by Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
Modified: 2011-07-16 11:37 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
without tests (51.60 KB, patch)
2011-04-11 12:37 PDT, Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
no flags Details | Diff | Splinter Review
updated (65.10 KB, patch)
2011-04-11 17:25 PDT, Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
no flags Details | Diff | Splinter Review
Unbitrotted (65.31 KB, patch)
2011-04-18 10:12 PDT, Wesley Johnston (:wesj)
no flags Details | Diff | Splinter Review
Unbitrotted (65.48 KB, patch)
2011-04-22 01:14 PDT, Olli Pettay [:smaug] (way behind * queues, especially ni? queue)
jst: review+
mbrubeck: feedback+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-08 10:32:35 PDT
...this way Fennec could dispatch touch events.

Implementing the interfaces shouldn't take too much time,
but sending the events via DOMWindowUtils may need a bit more time.

All this should be pref'ed so that desktop Fx doesn't expose any of this
to web content.
Comment 1 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-08 10:33:42 PDT
The event targeting is not quite clear yet in the spec, but we can
tweak that in followup bugs.
Comment 2 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-08 10:35:24 PDT
Actually, would it be enough to implement just the event interfaces now,
and add the DOMWindowUtils part later?
Matt?
Comment 3 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-08 10:36:34 PDT
...basically, could Fennec call
var e = document.createEvent("TouchEvent");
e.initTouchEvent(...):
someTarget.dispatchEvent(e);
Comment 4 Matt Brubeck (:mbrubeck) 2011-04-08 11:08:21 PDT
Yes, that would work for us.
Comment 5 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-11 12:37:23 PDT
Created attachment 525136 [details] [diff] [review]
without tests
Comment 6 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-11 13:06:51 PDT
The patch is still missing support for onFoo event listeners.
New patch with that fixed and tests coming soon.
Comment 7 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-11 17:25:32 PDT
Created attachment 525229 [details] [diff] [review]
updated
Comment 8 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-11 17:33:26 PDT
Comment on attachment 525229 [details] [diff] [review]
updated

Matt, or anyone, feel free to try this.
Hopefully it even compiles.

I posted the patch to tryserver.
Comment 9 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-11 17:36:17 PDT
Jst, the patch has the changes to nsDOMClassInfo and nsScriptNameSpaceManager
to allow us to pref off interfaces. Do those changes look reasonable to you?
It is a bit ugly hack to add the mDisabled flag, but other options
would probably need quite a bit re-factoring.
I'll ask review for this once it has compiled/passed tests on tryserver.
Comment 10 Wesley Johnston (:wesj) 2011-04-18 10:12:49 PDT
Created attachment 526751 [details] [diff] [review]
Unbitrotted

Needed to unbit rot this for testing.
Comment 11 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-22 01:14:03 PDT
Created attachment 527736 [details] [diff] [review]
Unbitrotted
Comment 12 Matt Brubeck (:mbrubeck) 2011-04-24 21:08:36 PDT
Comment on attachment 527736 [details] [diff] [review]
Unbitrotted

I don't see any major problems here.  Note that I'm mostly looking only at the IDL interfaces, not the C++ code.  Here are some notes about things that should change in either the code or the spec:

In the Editor's Draft spec, we have removed the timeStamp attribute from the TouchPoint interface.

I think we should either rename createTouch to createTouchPoint, or rename TouchPoint to Touch.  I personally prefer the latter.  For spec discussion, see: http://www.w3.org/2010/webevents/track/issues/11

We should remove the pageX/pageY parameters from createTouch.  These should be set automatically, as they are in initMouseEvent.

We will soon add a relatedTarget attribute to the TouchEvent interface and initTouchEvent function: http://www.w3.org/2010/webevents/track/issues/10
Comment 13 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-25 02:31:56 PDT
(In reply to comment #12) 
> In the Editor's Draft spec, we have removed the timeStamp attribute from the
> TouchPoint interface.
Will remove


> 
> I think we should either rename createTouch to createTouchPoint, or rename
> TouchPoint to Touch.  I personally prefer the latter.  For spec discussion,
> see: http://www.w3.org/2010/webevents/track/issues/11
Yeah, I think we just need to rename the interface


> 
> We should remove the pageX/pageY parameters from createTouch.  These should be
> set automatically, as they are in initMouseEvent.
Well, webkit's createTouch doesn't have clientX/Y but only pageX/Y and screenX/Y.
WebKit's implementation does not make any sense, but we may just have to do what they do.
What does Apple do?
I think I need to add automatic conversion from pageX/Y if clientX/Y are not passed as
parameters.

 
> We will soon add a relatedTarget attribute to the TouchEvent interface and
> initTouchEvent function: http://www.w3.org/2010/webevents/track/issues/10
I'm not 100% sure we want touchenter/leave because other touch events have different
event targeting (always the same target).
Again, IMHO, the special event targeting in touchup/down/move is just silly, but
that is what Apple and webkit do :(
Comment 14 Matt Brubeck (:mbrubeck) 2011-04-25 11:18:55 PDT
(In reply to comment #13)

> > We should remove the pageX/pageY parameters from createTouch.  These should be
> > set automatically, as they are in initMouseEvent.

> Well, webkit's createTouch doesn't have clientX/Y but only pageX/Y and
> screenX/Y.
> WebKit's implementation does not make any sense, but we may just have to do
> what they do.
> What does Apple do?

I think it's okay to break compatibility with createTouch (and the other create/init functions).  As far as I can tell, these are really used only in tests, not in public content (except "if (document.createTouch) ..." for feature detection).

> > We will soon add a relatedTarget attribute to the TouchEvent interface and
> > initTouchEvent function: http://www.w3.org/2010/webevents/track/issues/10
> I'm not 100% sure we want touchenter/leave because other touch events have
> different event targeting (always the same target).

I agree that the enter/leave events are a bit weird, but they solve a use case that is not easy to solve otherwise.  I'm not sure the targeting difference is a strong enough reason to leave them out.
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2011-04-25 12:06:13 PDT
Comment on attachment 527736 [details] [diff] [review]
Unbitrotted

Review of attachment 527736 [details] [diff] [review]:

Looks good to me, r=jst

::: content/base/src/nsDocument.cpp
@@ +8433,5 @@
+      if (valueType == nsIDataType::VTYPE_INTERFACE ||
+          valueType == nsIDataType::VTYPE_INTERFACE_IS) {
+        nsISupports** values = static_cast<nsISupports**>(rawArray);
+        for (PRUint32 i = 0; i < valueCount; ++i) {
+          nsISupports* supports = values[i];

Maybe use an nsCOMPtr<nsISupports> here?
Comment 16 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-26 04:45:09 PDT
Ah, yes,
nsCOMPtr<nsISupports> supports = dont_Addref(values[i]);
Comment 17 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-26 04:47:31 PDT
Matt, Wesley, I'm planning to push this without changes to interfaces
(except the removal of timestamp).
We can change the interfaces once they are defined/changed in the spec.
Touch event interfaces are disabled by default anyway.
Comment 18 Matt Brubeck (:mbrubeck) 2011-04-26 12:26:45 PDT
http://hg.mozilla.org/mozilla-central/rev/2bf4a6a4b551
Comment 19 :Ms2ger (⌚ UTC+1/+2) 2011-04-27 01:49:57 PDT
Smaug, at <http://hg.mozilla.org/mozilla-central/rev/2bf4a6a4b551#l9.184>, don't you need to initialize aRetVal if you dan't have a point with the given identifier?
Comment 20 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-04-27 04:50:32 PDT
In practice no, but that is a good habit to do.
Fixing...
Comment 21 Eric Shepherd [:sheppy] 2011-07-07 12:16:42 PDT
*** Bug 669777 has been marked as a duplicate of this bug. ***
Comment 22 Eric Shepherd [:sheppy] 2011-07-13 14:16:03 PDT
Added reference documentation. We need an example and other how-to information on the main en/DOM/Touch_events page still. Anyone have something they can contribute for that?

https://developer.mozilla.org/en/DOM/Touch_events

https://developer.mozilla.org/en/DOM/TouchEvent
https://developer.mozilla.org/en/DOM/TouchEvent.touches
https://developer.mozilla.org/en/DOM/TouchEvent.changedTouches
https://developer.mozilla.org/en/DOM/TouchEvent.targetTouches

https://developer.mozilla.org/en/DOM/DocumentTouch
https://developer.mozilla.org/en/DOM/DocumentTouch.createTouch
https://developer.mozilla.org/en/DOM/DocumentTouch.createTouchList

https://developer.mozilla.org/en/DOM/TouchList
https://developer.mozilla.org/en/DOM/TouchList.length
https://developer.mozilla.org/en/DOM/TouchList.identifiedTouch
https://developer.mozilla.org/en/DOM/TouchList.item

https://developer.mozilla.org/en/DOM/Touch
https://developer.mozilla.org/en/DOM/Touch.identifier
https://developer.mozilla.org/en/DOM/Touch.screenX
https://developer.mozilla.org/en/DOM/Touch.screenY
https://developer.mozilla.org/en/DOM/Touch.clientX
https://developer.mozilla.org/en/DOM/Touch.clientY
https://developer.mozilla.org/en/DOM/Touch.pageX
https://developer.mozilla.org/en/DOM/Touch.pageY
https://developer.mozilla.org/en/DOM/Touch.radiusX
https://developer.mozilla.org/en/DOM/Touch.radiusY
https://developer.mozilla.org/en/DOM/Touch.rotationAngle
https://developer.mozilla.org/en/DOM/Touch.force
Comment 23 Eric Shepherd [:sheppy] 2011-07-15 13:44:56 PDT
I have added an example and how-to guide to:

https://developer.mozilla.org/en/DOM/Touch_events

Documentation is complete.
Comment 24 Wilson Guaraca 2011-07-15 13:55:37 PDT
Thank you for your help. We have decided to postpone the Dev Derby competition focused around Touch because our mobile browser doesn't yet fully support it. We will feature this technology as soon as it becomes available. Thank you for your help and all this information you have gathered for us will be useful when we do decide to run this competition. 

Thank you!
Comment 25 Eric Shepherd [:sheppy] 2011-07-16 11:37:54 PDT
Needed the docs anyway. I didn't realize Dev Derby was planning something around it. Heh.

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