Bug 149676 – Implement keyboard event sending for iOS in WebKitTestRunner

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Wenson Hsieh]

Get WKTR to be able to generate and send keyboard events on iOS.

[Wenson Hsieh]

Created attachment 262195 [details]
Patch

[Wenson Hsieh]

Created attachment 262196 [details]
Patch

[Tim Horton]

Comment on attachment 262196 [details]
Patch

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

> Tools/WebKitTestRunner/ios/HIDEventGenerator.h:54
> +- (void)keyDown:(NSString*)character completionBlock:(void (^)(void))completionBlock;

space before the star.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:443
> +    if ([key length] != 1)

dot?

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:481
> +    if ([key length] == 1) {

dot.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:495
> +        case '`':

Is there really nothing in the system that can do this for us (even if it's SPI?).

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:98
> +    TestRunnerWKWebView* webView = TestController::singleton().mainWebView()->platformView();

star's on the wrong side

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:100
> +    NSString* characterAsNSString = [[NSString stringWithCString:toSTD(toWK(character)).c_str()] autorelease];

I'm sure there's a better way to get a NSString from a WKString. Going through std::string is crazy. At the very least, use WKStringGetUTF8CString instead of std::string::c_str(), no?

Also, star.

[Wenson Hsieh]

Comment on attachment 262196 [details]
Patch

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

Thanks for taking a look! I'll address the comments and get the bots to build (looks like I need to link against the HID usage tables). I'll have a new patch up soon.

>> Tools/WebKitTestRunner/ios/HIDEventGenerator.h:54
>> +- (void)keyDown:(NSString*)character completionBlock:(void (^)(void))completionBlock;
> 
> space before the star.

Got it, I'll change this.

>> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:443
>> +    if ([key length] != 1)
> 
> dot?

Got it. I'll use dot for accessing properties from now on.

>> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:481
>> +    if ([key length] == 1) {
> 
> dot.

Will fix!

>> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:495
>> +        case '`':
> 
> Is there really nothing in the system that can do this for us (even if it's SPI?).

I found nothing that could convert the non-alphanums (!@#$%, etc.) to their respective usage keys :(

>> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:98
>> +    TestRunnerWKWebView* webView = TestController::singleton().mainWebView()->platformView();
> 
> star's on the wrong side

Will fix!

>> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:100
>> +    NSString* characterAsNSString = [[NSString stringWithCString:toSTD(toWK(character)).c_str()] autorelease];
> 
> I'm sure there's a better way to get a NSString from a WKString. Going through std::string is crazy. At the very least, use WKStringGetUTF8CString instead of std::string::c_str(), no?
> 
> Also, star.

I'll look for a cleaner way to do this.

Also, will fix the star.

[Simon Fraser (smfr)]

Comment on attachment 262196 [details]
Patch

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

> Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:38
> +    void typeCharacter(DOMString character, object callback);

This should be named to make it clear whether it's a hardware keyboard event, or a software keyboard event.

> Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:39
> +

It may make sense to add callback properties for keyboard hide/show.

> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.cpp:88
> +#if PLATFORM(IOS)
> +void UIScriptController::setDidShowKeyboardCallback(JSValueRef callback)
> +{
> +    m_didShowKeyboardCallback = m_context.registerCallback(callback);
> +    platformSetDidShowKeyboardCallback();
> +}
> +
> +JSValueRef UIScriptController::didShowKeyboardCallback() const
> +{
> +    return m_context.callbackWithID(m_didShowKeyboardCallback);
> +}
> +#endif

This should go in UIScriptControllerIOS.mm

> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.cpp:103
> +void typeCharacter(JSStringRef, JSValueRef)

UIScriptController::

> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.h:62
> +#if PLATFORM(IOS)
> +    void setDidShowKeyboardCallback(JSValueRef);
> +    JSValueRef didShowKeyboardCallback() const;
> +#endif

I've avoided #idfefs here, and just stubbed the functions out for non-iOS.

> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.h:79
> +#if PLATFORM(IOS)
> +    void platformSetDidShowKeyboardCallback();
> +#endif

Ditto.

> Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:66
> +        _keyboardMayAcceptKeyInput = NO;

The name of this property is confusing. Is it really _keyboardIsVisible?

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:139
> +    [self _sendHIDEvent:IOHIDEventCreateKeyboardEvent(kCFAllocatorDefault,
> +        timestamp,
> +        kHIDPage_KeyboardOrKeypad,
> +        usage,
> +        isKeyDown,
> +        kIOHIDEventOptionNone)];

You're leaking the event.

[Wenson Hsieh]

Comment on attachment 262196 [details]
Patch

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

>> Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:38
>> +    void typeCharacter(DOMString character, object callback);
> 
> This should be named to make it clear whether it's a hardware keyboard event, or a software keyboard event.

Renaming to typeCharacterUsingHardwareKeyboard

>> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.cpp:103
>> +void typeCharacter(JSStringRef, JSValueRef)
> 
> UIScriptController::

Fixed.

>> Tools/WebKitTestRunner/UIScriptContext/UIScriptController.h:62
>> +#endif
> 
> I've avoided #idfefs here, and just stubbed the functions out for non-iOS.

Got it. I'll stub out the functions for non-iOS.

>> Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:66
>> +        _keyboardMayAcceptKeyInput = NO;
> 
> The name of this property is confusing. Is it really _keyboardIsVisible?

Yes, keyboardIsVisible would be a more accurate name. I'll change it to that.

>> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:139
>> +        kIOHIDEventOptionNone)];
> 
> You're leaking the event.

Good catch. Adding temp RetainPtr so the event is released after sending.

[Wenson Hsieh]

Created attachment 262231 [details]
Patch

[WebKit Commit Bot]

Attachment 262231 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:135:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:136:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:137:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:138:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:139:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:140:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:141:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:142:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:143:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:144:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:145:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:146:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:147:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:148:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:149:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:150:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:151:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:152:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:153:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:154:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:155:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:156:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:157:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:158:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:159:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:160:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:161:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:162:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:163:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:164:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:165:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:166:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:167:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:168:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:169:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:170:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:171:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:172:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 39 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Wenson Hsieh]

Created attachment 262232 [details]
Patch

[WebKit Commit Bot]

Attachment 262232 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:135:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:136:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:137:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:138:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:139:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:140:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:141:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:142:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:143:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:144:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:145:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:146:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:147:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:148:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:149:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:150:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:151:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:152:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:153:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:154:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:155:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:156:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:157:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:158:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:159:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:160:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:161:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:162:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:163:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:164:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:165:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:166:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:167:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:168:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:169:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:170:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:171:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Tools/WebKitTestRunner/ios/IOKitSPI.h:172:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 39 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 262232 [details]
Patch

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

> Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:66
> +        NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
> +        [center addObserver:self selector:@selector(_keyboardDidShow:) name:UIKeyboardDidShowNotification object:nil];
> +        [center addObserver:self selector:@selector(_keyboardDidHide:) name:UIKeyboardDidHideNotification object:nil];

You need to call removeObserver in -dealloc.

> Tools/WebKitTestRunner/ios/IOKitSPI.h:131
> +    kHIDPage_KeyboardOrKeypad = 0x07

Move this up above kHIDPage_VendorDefinedStart.

[Wenson Hsieh]

Committed r190407: <http://trac.webkit.org/changeset/190407>