Bug 97326 – [WK2][WKTR] EventSender needs to implement scheduleAsynchronousClick

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

WebKitTestRunner's EventSender does not implement scheduleAsynchronousClick(), causing the following test case to fail:
  fast/events/popup-blocking-click-in-iframe.html

[Chris Dumez]

Created attachment 165111 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 165111 [details]
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:240
> +    WKRetainPtr<WKStringRef> EventSenderMessageName(AdoptWK, WKStringCreateWithUTF8CString("EventSender"));
> +    WKRetainPtr<WKMutableDictionaryRef> EventSenderMessageBody(AdoptWK, WKMutableDictionaryCreate());

Nit: geolocation etc use lowercase messages, maybe we want to be consistent

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:250
> +    WKRetainPtr<WKStringRef> subMessageKey(AdoptWK, WKStringCreateWithUTF8CString("SubMessage"));

Is submessage really a good way to describe this?

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:253
> +    WKBundlePostMessage(InjectedBundle::shared().bundle(), EventSenderMessageName.get(), EventSenderMessageBody.get());

I would add a newline after post

> Tools/WebKitTestRunner/TestController.cpp:694
> +#if PLATFORM(MAC) || PLATFORM(QT) || PLATFORM(GTK) || PLATFORM(EFL)

So which webkit2 platforms does this not work on?

[Chris Dumez]

Comment on attachment 165111 [details]
Patch

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

>> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:240
>> +    WKRetainPtr<WKMutableDictionaryRef> EventSenderMessageBody(AdoptWK, WKMutableDictionaryCreate());
> 
> Nit: geolocation etc use lowercase messages, maybe we want to be consistent

Those messages already exists and their format is already predefined. I cannot change them. Those messages are used already in EventSendingController::mouseDown() and EventSendingController::mouseUp().

>> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:250
>> +    WKRetainPtr<WKStringRef> subMessageKey(AdoptWK, WKStringCreateWithUTF8CString("SubMessage"));
> 
> Is submessage really a good way to describe this?

Ditto.

>> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:253
>> +    WKBundlePostMessage(InjectedBundle::shared().bundle(), EventSenderMessageName.get(), EventSenderMessageBody.get());
> 
> I would add a newline after post

Ok.

>> Tools/WebKitTestRunner/TestController.cpp:694
>> +#if PLATFORM(MAC) || PLATFORM(QT) || PLATFORM(GTK) || PLATFORM(EFL)
> 
> So which webkit2 platforms does this not work on?

I used this to be consistent with TestController::didReceiveSynchronousMessageFromInjectedBundle().

[Chris Dumez]

Comment on attachment 165111 [details]
Patch

I think I can do some refactoring to avoid patch duplication.

[Chris Dumez]

Created attachment 165120 [details]
Patch

- Add createMouseMessageBody() convenience function to avoid code duplication for creating mouse messages
- Add newline as suggested by Kenneth

[Kenneth Rohde Christiansen]

Comment on attachment 165120 [details]
Patch

nice

[WebKit Review Bot]

Comment on attachment 165120 [details]
Patch

Clearing flags on attachment: 165120

Committed r129220: <http://trac.webkit.org/changeset/129220>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.