Bug 149676

Summary:

Implement keyboard event sending for iOS in WebKitTestRunner

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, enrica, lforschler, simon.fraser, thorton

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

iOS 9.0

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	simon.fraser: review+

Wenson Hsieh

Reported 2015-09-30 13:00:28 PDT

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

Attachments
Patch (20.86 KB, patch) 2015-09-30 15:19 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Patch (20.68 KB, patch) 2015-09-30 15:22 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Patch (23.58 KB, patch) 2015-09-30 21:58 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Patch (23.35 KB, patch) 2015-09-30 22:12 PDT, Wenson Hsieh	simon.fraser: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2015-09-30 15:19:45 PDT

Created attachment 262195 [details] Patch

Wenson Hsieh

Comment 2 2015-09-30 15:22:20 PDT

Created attachment 262196 [details] Patch

Tim Horton

Comment 3 2015-09-30 15:46:51 PDT

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 4 2015-09-30 16:03:00 PDT

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 5 2015-09-30 16:28:01 PDT

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 6 2015-09-30 16:37:52 PDT

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

Comment 7 2015-09-30 21:58:11 PDT

Created attachment 262231 [details] Patch

WebKit Commit Bot

Comment 8 2015-09-30 22:00:25 PDT

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

Comment 9 2015-09-30 22:12:09 PDT

Created attachment 262232 [details] Patch

WebKit Commit Bot

Comment 10 2015-09-30 22:13:50 PDT

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 11 2015-09-30 22:57:44 PDT

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

Comment 12 2015-10-01 07:29:08 PDT

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

Note You need to log in before you can comment on or make changes to this bug.