Bug 59030 – Make WebKit expose extra touch information

• Component: WebCore Misc.
• Status: RESOLVED FIXED

Some touch devices provide, other than (x,y) location, extra touch information, like the major/minor axis length, and  
the orientation of the touch ellipse. Attached is a proposed patch that routes these extra touch information through
chromium port to WebKit and exposes them to DOM.

WebCore::Touch is augmented with three fields :
m_radius ,  the major axis length of the touch ellipse
m_ratio  ,  the ratio between major and minor axis length of the touch ellipse
m_angle  ,  the orientation of the touch ellipse


The extra touch information is passed in from chromium's representation of touch event
WebKit::WebTouchPoint and then routed to platform touch event WebKit::PlatformTouchEventBuilder

Touch.idl and Document.idl are modified to expose these extra fields to DOM.

Created attachment 90431 [details]
Patch

update cc list

[Darin Fisher (:fishd, Google)]

Comment on attachment 90431 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90431&action=review

> Source/WebCore/dom/Touch.idl:39
> +        readonly attribute float            radius;

what specification covers these additions?  if none, then shouldn't
we vendor prefix these to avoid possible collisions in the future
with a formal specification that may define these fields differently?

i confess that i don't know much about the history of this file.
is there already a spec covering the existing fields?

[Alexey Proskuryakov]

If there is no specification, then adding this code to WebKit even under an #if and even with a vendor prefix shouldn't be taken lightly.

Also, this patch has style errors that the bot didn't find.

Created attachment 90568 [details]
Patch

(In reply to comment #3)
> (From update of attachment 90431 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90431&action=review
> 
> > Source/WebCore/dom/Touch.idl:39
> > +        readonly attribute float            radius;
> 
> what specification covers these additions?  if none, then shouldn't
> we vendor prefix these to avoid possible collisions in the future
> with a formal specification that may define these fields differently?
> 
> i confess that i don't know much about the history of this file.
> is there already a spec covering the existing fields?

W3C has a Draft of the "Touch Events Specification" at

http://dvcs.w3.org/hg/webevents/raw-file/tip/touchevents.html

It is largely compatible with the touch event interface used in the Safari
DOM model (and the API exposed by WebKit). 

Also the draft is going to add some extra properties for touch point,
including radiusX, radiusY (the X and Y axis of the touch ellipse) and
roatationAngle, which is covered by this patch.

Even Safari/WebKit is not going to follow the draft spec (which is true
currently since the Safari/WebKit DOM model is already there while the 
spec is still being drafted, and also note that Mozilla exposes different
set of API for the touch events), it is still desirable to have the features
to expose these touch information.

(In reply to comment #4)
> If there is no specification, then adding this code to WebKit even under an #if and even with a vendor prefix shouldn't be taken lightly.
>

Please see the comment #6.

 
> Also, this patch has style errors that the bot didn't find.

Would you mind pointing me where it is ?

(In reply to comment #5)
> Created an attachment (id=90568) [details]
> Patch


The extra touch information are changed to comply with the draft spec  

http://dvcs.w3.org/hg/webevents/raw-file/tip/touchevents.html

Now the WebCore::Touch is augmented with 3 fields :

m_radiusX
m_radiusY
m_rotationAngle

[Alexey Proskuryakov]

Comment on attachment 90568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90568&action=review

I was also going to complain about text alignment (we normally use one space), but those are all pre-exisintg issues in this code.

I don't object to landing this, given the kind of support there already is for touch events.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

The patch can't be landed with this line. You should add tests.

> Source/WebKit/chromium/public/WebTouchPoint.h:52
> +        , rotationAngle(0.0) { }

Just "0", and the braces should go on separate lines.

Created attachment 91147 [details]
Patch

Comment on attachment 90568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90568&action=review

>> Source/WebCore/ChangeLog:8
>> +        No new tests. (OOPS!)
> 
> The patch can't be landed with this line. You should add tests.

done.

>> Source/WebKit/chromium/public/WebTouchPoint.h:52
>> +        , rotationAngle(0.0) { }
> 
> Just "0", and the braces should go on separate lines.

done

tests added.

(In reply to comment #10)
> Created an attachment (id=91147) [details]
> Patch

Created attachment 92277 [details]
Patch

(In reply to comment #13)
> Created an attachment (id=92277) [details]
> Patch

prefix the three fields of WebCore::Touch

radiusX
radiusY
rotationAngle

to be 

webkitRadiusX
webkitRadiusY
webkitRotationAngle

for now and will change to the standard name when the draft 

http://dvcs.w3.org/hg/webevents/raw-file/tip/touchevents.html#idl-def-Touch

is approved.

[Darin Fisher (:fishd, Google)]

Well, hmm...

It looks like that spec also covers the other pre-existing fields of Touch.  It looks like the spec is co-edited by folks from Mozilla and Opera.  Do you know the state of their implementations?  Have they vendor prefixed these fields?  Have they been shipping these fields for some time already?

Comment #6 makes me concerned that there may be considerable divergence between the browsers.

(In reply to comment #15)
> Well, hmm...
> 
> It looks like that spec also covers the other pre-existing fields of Touch.  It looks like the spec is co-edited by folks from Mozilla and Opera.  Do you know the state of their implementations?  Have they vendor prefixed these fields?  Have they been shipping these fields for some time already?
> 
> Comment #6 makes me concerned that there may be considerable divergence between the browsers.


According to this thread

http://lists.w3.org/Archives/Public/public-webevents/2011AprJun/0083.html

Matt Brubeck from Mozilla, also one of the editors of the draft, has an
experimental implementation in the nightly build of mobile Firefox.

Also Mozilla has vendor prefixed MozTouchEvent and related APIs which are
not compatible with WebKit.

Opera's implementation is based on iphone (and so the same as WebKit)

The draft is based on WebKit's implementation but some divergence is 
developing.

As for the radius and angle information, since they are now available
from many touch devices, it is good to expose them to web developers.
And as the draft is still evolving, vendor-prefixing the fields seems the
safe way to go for now.

Hi Darin,

more thoughts on this patch ?

[Darin Fisher (:fishd, Google)]

Comment on attachment 92277 [details]
Patch

Agreed.  Sorry for the high latency here.  I was out on vacation.

[WebKit Commit Bot]

Comment on attachment 92277 [details]
Patch

Clearing flags on attachment: 92277

Committed r86700: <http://trac.webkit.org/changeset/86700>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.