[webkit-reviews] review denied: [Bug 98410] [WK2][WKTR] WebKitTestRunner's eventSender.contextClick() returns objects without implemented click() method : [Attachment 203846] click() method for objects returned by eventSender.clontextClick()

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 17 14:32:40 PDT 2013


Benjamin Poulain <benjamin at webkit.org> has denied Wojciech Bielawski
<w.bielawski at samsung.com>'s request for review:
Bug 98410: [WK2][WKTR] WebKitTestRunner's eventSender.contextClick() returns
objects without implemented click() method
https://bugs.webkit.org/show_bug.cgi?id=98410

Attachment 203846: click() method for objects returned by
eventSender.clontextClick()
https://bugs.webkit.org/attachment.cgi?id=203846&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=203846&action=review


Looks good. Please enable on OS X and check the comments bellow.

Interestingly, this API does not exist on Mac or Qt.
Given that DRT returns a list of string on OS X and Qt, I think your JSObject
should have toString() that returns the item's title.

> LayoutTests/ChangeLog:13
> +	   * platform/efl-wk2/TestExpectations: Corrected bug number for tests:

> +	    editing/pasteboard/can-read-in-copy-and-cut-events.html
> +	    editing/pasteboard/can-read-in-dragstart-event.html
> +	   * platform/gtk-wk2/TestExpectations:

Please at least enable Mac. The EWS bots will help you find what test changes
as a result.

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:46
> +    // This object has to own menu item because it is not possible to inform
JavaScript when context menu is closing.

This comment is misleading. Please remove it.

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:49
> +    MenuItemPrivateData(WKBundlePageRef page, WKContextMenuItemRef item) :
m_page(page), m_item(item) { }

Style:
-Make this the first member of the definition.
-Split the initialization over multiple lines.


More information about the webkit-reviews mailing list