The Document object is missing support for the createTouch and createTouchList APIs, as defined here: http://developer.apple.com/library/safari/#documentation/UserExperience/Reference/DocumentAdditionsReference/DocumentAdditions/DocumentAdditions.html Many sites (including New York Times) use these APIs (especially createTouch) to determine if the browser has support for touch events. Patch and layout test to follow.
Created attachment 70877 [details] Patch and layout tests.
Comment on attachment 70877 [details] Patch and layout tests. View in context: https://bugs.webkit.org/attachment.cgi?id=70877&action=review > WebCore/dom/Document.cpp:4775 > + Frame* frame = window ? window->frame() : this->frame(); Why 'this'? > WebCore/dom/Document.idl:326 > + in Node target, Shouldn't this be of type EventTarget? > WebCore/dom/Document.idl:332 > + raises (DOMException); Should perhaps be on the line above? > WebCore/dom/Document.idl:334 > + raises (DOMException); Can be on one line? > LayoutTests/fast/events/touch/script-tests/document-create-touch.js:12 > +var touch = document.createTouch(window, target, 1, 100, 101, 102, 103); When should the method throw an exception? Can you add some test cases for this? We should also be sure to test passing the wrong argument types.
> > WebCore/dom/Document.cpp:4775 > > + Frame* frame = window ? window->frame() : this->frame(); > > Why 'this'? > The compiler didn't like using a local that had the same name as a member function, and 'this' resolved the ambiguity. I did think about changing the name of the local first, but frame seemed most suitable. However I can use a different name if you'd prefer that over using 'this'. > > WebCore/dom/Document.idl:326 > > + in Node target, > > Shouldn't this be of type EventTarget? > Yes, you're right. I'll correct that and send a new patch. > > WebCore/dom/Document.idl:332 > > + raises (DOMException); > > Should perhaps be on the line above? > > > WebCore/dom/Document.idl:334 > > + raises (DOMException); > > Can be on one line? Judging by the style of the rest of the file, raises(...) seems to always go on the next line indented by 4 spaces. > > > LayoutTests/fast/events/touch/script-tests/document-create-touch.js:12 > > +var touch = document.createTouch(window, target, 1, 100, 101, 102, 103); > > When should the method throw an exception? Can you add some test cases for this? We should also be sure to test passing the wrong argument types. The documentation doesn't mention when the methods would throw. I tried a few things on the iPod Touch, but couldn't get it to throw. I can add an extra test to pass incorrect argument types, but I think that type checking is taken care of by the generated portion of the bindings so I would've thought it was already quite well tested? Thanks, Ben
Created attachment 71035 [details] Patch and layout tests v2
Comment on attachment 71035 [details] Patch and layout tests v2 Looks good to me. Simon, do you have any comments before this is submitted?
Comment on attachment 71035 [details] Patch and layout tests v2 I think we can submit now, given that we have https://bugs.webkit.org/show_bug.cgi?id=47819 to track any changes required to get the required exception behaviour.
Comment on attachment 71035 [details] Patch and layout tests v2 Clearing flags on attachment: 71035 Committed r70047: <http://trac.webkit.org/changeset/70047>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/70047 might have broken Qt Linux Release The following tests are not passing: fast/dom/Window/window-properties.html
(In reply to comment #9) > http://trac.webkit.org/changeset/70047 might have broken Qt Linux Release > The following tests are not passing: > fast/dom/Window/window-properties.html Oops, forgot the update to the expected output. Patch on the way.
http://trac.webkit.org/changeset/70052 should take care of it.