Attachment 356394 Details for Bug 191608 – To Land

[patch] To Land

bug-191608-20181203124303.patch (text/plain), 16.67 KB, created by Daniel Bates on 2018-12-03 12:43:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-12-03 12:43:04 PST

Size: 16.67 KB

patch

obsolete

>Subversion Revision: 238797
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index bbd98215365f216a76b9d8433e7cc8984bdf3d22..70a2f81e20b950717ae575b3132cbf98eeedea4b 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-12-03  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Do not handle key events that are key commands
>+        https://bugs.webkit.org/show_bug.cgi?id=191608
>+        <rdar://problem/46046013>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        A key down event may be associated with a key command. If it is then we want to execute the
>+        key command instead of inserting or deleting text. We need to ask UIKit to handle the current
>+        event as a key command to find out.
>+
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView _interpretKeyEvent:isCharEvent:]): Ask UIKit to handle the current event
>+        as a key command. If it handles it then we're done. Otherwise, do what we do now.
>+
> 2018-12-03  Darin Adler  <darin@apple.com>
> 
>         [Cocoa] Streamline sandbox code a tiny bit
>diff --git a/Source/WebKitLegacy/ios/ChangeLog b/Source/WebKitLegacy/ios/ChangeLog
>index 45226fd4d494d579f563ce2d0afe7be6b0b3527b..b5e32fe365ded05f08a016ce88028da37d25fb1e 100644
>--- a/Source/WebKitLegacy/ios/ChangeLog
>+++ b/Source/WebKitLegacy/ios/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-12-03  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Do not handle key events that are key commands
>+        https://bugs.webkit.org/show_bug.cgi?id=191608
>+        <rdar://problem/46046013>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Add default implementation of -handleKeyCommandForCurrentEvent that returns NO - the current
>+        event was not handled as a key command.
>+
>+        * DefaultDelegates/WebDefaultUIKitDelegate.m:
>+        (-[WebDefaultUIKitDelegate handleKeyCommandForCurrentEvent]):
>+        * WebView/WebUIKitDelegate.h:
>+
> 2018-11-29  Zalan Bujtas  <zalan@apple.com>
> 
>         Rename *ObservedContentModifier(s) to *ObservedDOMTimer(s)
>diff --git a/Source/WebKitLegacy/mac/ChangeLog b/Source/WebKitLegacy/mac/ChangeLog
>index 1efaf52cc017ca68897afe8584b56de5e7b2d674..56f462840026e742359a3738eee8e21e8478baba 100644
>--- a/Source/WebKitLegacy/mac/ChangeLog
>+++ b/Source/WebKitLegacy/mac/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-12-03  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Do not handle key events that are key commands
>+        https://bugs.webkit.org/show_bug.cgi?id=191608
>+        <rdar://problem/46046013>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        A key down event may be associated with a key command. If it is then we want to execute the
>+        key command instead of inserting or deleting text. We need to ask UIKit to handle the current
>+        event as a key command to find out.
>+
>+        * WebView/WebHTMLView.mm:
>+        (-[WebHTMLView _handleEditingKeyEvent:]):
>+
> 2018-12-02  Zalan Bujtas  <zalan@apple.com>
> 
>         Add a runtime feature flag for LayoutFormattingContext.
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index e6114c721988fcd42eb914258f5c46df8d5289f5..9a843e0bdac0a267056ce49ae7c75259e764d5d5 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -133,6 +133,14 @@ @end
> 
> #endif
> 
>+#if PLATFORM(IOS_FAMILY)
>+
>+@interface UIKeyboardImpl (Staging)
>+- (BOOL)handleKeyCommandForCurrentEvent;
>+@end
>+
>+#endif
>+
> using namespace WebCore;
> using namespace WebKit;
> 
>@@ -3963,8 +3971,13 @@ - (BOOL)_interpretKeyEvent:(::WebEvent *)event isCharEvent:(BOOL)isCharEvent
>         return YES;
> 
>     UIKeyboardImpl *keyboard = [UIKeyboardImpl sharedInstance];
>+
>+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000
>+    if (event.type == WebEventKeyDown && [keyboard respondsToSelector:@selector(handleKeyCommandForCurrentEvent)] && [keyboard handleKeyCommandForCurrentEvent])
>+        return YES;
>+#endif
>+
>     NSString *characters = event.characters;
>-    
>     if (!characters.length)
>         return NO;
> 
>diff --git a/Source/WebKitLegacy/ios/DefaultDelegates/WebDefaultUIKitDelegate.m b/Source/WebKitLegacy/ios/DefaultDelegates/WebDefaultUIKitDelegate.m
>index dd3c835af763e60192dc44b0042cb1d1004b0c2f..aaee826c3339110583a1da60024f34e217233f9f 100644
>--- a/Source/WebKitLegacy/ios/DefaultDelegates/WebDefaultUIKitDelegate.m
>+++ b/Source/WebKitLegacy/ios/DefaultDelegates/WebDefaultUIKitDelegate.m
>@@ -162,6 +162,11 @@ - (void)webView:(WebView *)aWebView didReceiveMessage:(NSDictionary *)aMessage
> {
> }
> 
>+- (BOOL)handleKeyCommandForCurrentEvent
>+{
>+    return NO;
>+}
>+
> - (void)addInputString:(NSString *)str withFlags:(NSUInteger)flags
> {
> }
>diff --git a/Source/WebKitLegacy/ios/WebView/WebUIKitDelegate.h b/Source/WebKitLegacy/ios/WebView/WebUIKitDelegate.h
>index 3a4462e43889ec7b836fc276fbe7748bb193cd67..b85547b8fe6e2362c306df33f7c46eb51f8a9dd8 100644
>--- a/Source/WebKitLegacy/ios/WebView/WebUIKitDelegate.h
>+++ b/Source/WebKitLegacy/ios/WebView/WebUIKitDelegate.h
>@@ -90,6 +90,7 @@ typedef NS_ENUM(NSInteger, WebMediaCaptureType) {
> - (void)webView:(WebView *)webView didHideFullScreenForPlugInView:(id)plugInView;
> - (void)webView:(WebView *)aWebView didReceiveMessage:(NSDictionary *)aMessage;
> - (void)addInputString:(NSString *)str withFlags:(NSUInteger)flags;
>+- (BOOL)handleKeyCommandForCurrentEvent;
> // FIXME: remove deleteFromInput when UIKit implements deleteFromInputWithFlags.
> - (void)deleteFromInput;
> - (void)deleteFromInputWithFlags:(NSUInteger)flags;
>diff --git a/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm b/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm
>index 88a9e4b175e2d9d02f6741b746c62f18f58278ab..7bc3a29db47efc98cbe22cf2e22faf48560a089f 100644
>--- a/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm
>+++ b/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm
>@@ -6082,13 +6082,19 @@ - (BOOL)_handleEditingKeyEvent:(KeyboardEvent *)wcEvent
>         WebEvent *event = platformEvent->event();
>         if (event.keyboardFlags & WebEventKeyboardInputModifierFlagsChanged)
>             return NO;
>-        if (![[self _webView] isEditable] && event.isTabKey) 
>+
>+        WebView *webView = [self _webView];
>+        if (!webView.isEditable && event.isTabKey)
>             return NO;
>-        
>+
>+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000
>+        if (event.type == WebEventKeyDown && [webView._UIKitDelegateForwarder handleKeyCommandForCurrentEvent])
>+            return YES;
>+#endif
>+
>         NSString *s = [event characters];
>         if (!s.length)
>             return NO;
>-        WebView* webView = [self _webView];
>         switch ([s characterAtIndex:0]) {
>         case kWebBackspaceKey:
>         case kWebDeleteKey:
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 36eb9af945ff75219ecd9794ec2fd3eb55201a69..47ddee97f34eeeca871a22c6c439a3a8b29c0405 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-12-03  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Do not handle key events that are key commands
>+        https://bugs.webkit.org/show_bug.cgi?id=191608
>+        <rdar://problem/46046013>
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Add tests to ensure that we process key commands correctly.
>+
>+        * fast/events/ios/key-command-italic-dispatches-keydown-expected.txt: Added.
>+        * fast/events/ios/key-command-italic-dispatches-keydown.html: Added.
>+        * fast/events/ios/key-command-italic-expected.txt: Added.
>+        * fast/events/ios/key-command-italic.html: Added.
>+        * fast/events/ios/type-digits-holding-control-key-expected.txt: Added.
>+        * fast/events/ios/type-digits-holding-control-key.html: Added.
>+        * platform/ios-wk1/TestExpectations:
>+
> 2018-12-03  Eric Carlson  <eric.carlson@apple.com>
> 
>         [MediaStream] 'devicechange' event when more capture device information are revealed.
>diff --git a/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown-expected.txt b/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..ca301c87243750e28649fd4eda7bbc76c18bc0f8
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown-expected.txt
>@@ -0,0 +1,11 @@
>+Tests that pressing Command + I in a rich editing field dispatches a key down event. To run this test by hand, select all the text below and press Command + I.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS window.event.key is "i"
>+PASS window.event.metaKey is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown.html b/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..76bc63a446fb66cc291925d691bd6261948fa87c
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/key-command-italic-dispatches-keydown.html
>@@ -0,0 +1,78 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width">
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+<style>
>+#test {
>+    border: 1px solid black;
>+    height: 500px;
>+    width: 500px;
>+}
>+
>+.hidden {
>+    display: none;
>+}
>+</style>
>+</head>
>+<body>
>+<p id="description"></p>
>+<div id="test" contenteditable="true">This text should be italicized.</div>
>+<div id="console"></div>
>+<script>
>+window.jsTestIsAsync = true;
>+
>+let testElement;
>+let event;
>+let ignoredFirstKeyDownEvent = false;
>+
>+function shouldIgnoreKeyDownEvent(event)
>+{
>+    // When performing the test manually a person is not fast enough to press Command + I simultaneously.
>+    // We receive a key down for Command followed by a key down for Command + I. So, we ignore the first
>+    // event to normalize the test result.
>+    if (window.testRunner || ignoredFirstKeyDownEvent)
>+        return false;
>+    ignoredFirstKeyDownEvent = true;
>+    console.assert(event.key == "Meta");
>+    return true;
>+}
>+
>+function handleKeyDownEvent(event)
>+{
>+    if (shouldIgnoreKeyDownEvent(event))
>+        return;
>+
>+    testElement.removeEventListener("keydown", handleKeyDownEvent, true);
>+
>+    shouldBeEqualToString("window.event.key", "i");
>+    shouldBeTrue("window.event.metaKey");
>+
>+    // Remove the test element to make the output pretty.
>+    document.body.removeChild(document.getElementById("test"));
>+
>+    finishJSTest();
>+}
>+
>+function runTest()
>+{
>+    if (!window.testRunner)
>+        return;
>+    function handleFocus() {
>+        window.getSelection().selectAllChildren(testElement);
>+        UIHelper.keyDown("i", ["metaKey"]);
>+    }
>+    testElement.addEventListener("focus", handleFocus, { once: true });
>+    UIHelper.activateElement(testElement);
>+}
>+
>+description("Tests that pressing Command + I in a rich editing field dispatches a key down event. To run this test by hand, select all the text below and press Command + I.");
>+
>+testElement = document.getElementById("test");
>+testElement.addEventListener("keydown", handleKeyDownEvent, true);
>+
>+runTest();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/ios/key-command-italic-expected.txt b/LayoutTests/fast/events/ios/key-command-italic-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..805ce51cdf28ff6367684ff51e5b7e6ac1bc512d
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/key-command-italic-expected.txt
>@@ -0,0 +1,3 @@
>+Tests that pressing Command + I in a rich editing field italizes the selection.
>+| <i>
>+|   "<#selection-anchor>This text should be italicized.<#selection-focus>"
>diff --git a/LayoutTests/fast/events/ios/key-command-italic.html b/LayoutTests/fast/events/ios/key-command-italic.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..6d6a0a54b48596d0e93e8747870341851b0cee49
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/key-command-italic.html
>@@ -0,0 +1,58 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width">
>+<script src="../../../resources/ui-helper.js"></script>
>+<script src="../../../resources/dump-as-markup.js"></script>
>+<style>
>+#test {
>+    border: 1px solid black;
>+    height: 500px;
>+    width: 500px;
>+}
>+
>+.hidden {
>+    display: none;
>+}
>+</style>
>+</head>
>+<body>
>+<p id="description">Tests that pressing Command + I in a rich editing field italizes the selection.</p>
>+<p id="manual-instructions" class="hide">To run this test by hand, select all the text below and press Command + I. This test PASSED if the text becomes italic. Otherwise, it FAILED.</p>
>+<div id="test" contenteditable="true">This text should be italicized.</div>
>+<script>
>+let testElement = document.getElementById("test");
>+let mutationObserver = null;
>+
>+function handleChildListModified()
>+{
>+    mutationObserver.disconnect();
>+    Markup.dump(testElement);
>+    Markup.notifyDone();
>+}
>+
>+function runTest()
>+{
>+    if (!window.testRunner) {
>+        document.getElementById("manual-instructions").classList.remove("hidden");
>+        return;
>+    }
>+
>+    Markup.description(document.getElementById("description").textContent);
>+
>+    mutationObserver = new MutationObserver(handleChildListModified);
>+    mutationObserver.observe(testElement, { childList: true });
>+
>+    function handleFocus() {
>+        window.getSelection().selectAllChildren(testElement);
>+        UIHelper.keyDown("i", ["metaKey"]);
>+    }
>+    testElement.addEventListener("focus", handleFocus, { once: true });
>+    UIHelper.activateElement(testElement);
>+}
>+
>+Markup.waitUntilDone();
>+runTest();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/ios/type-digits-holding-control-key-expected.txt b/LayoutTests/fast/events/ios/type-digits-holding-control-key-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..56f7e4b041a2c68ded85cc72f1150714c90ffb70
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/type-digits-holding-control-key-expected.txt
>@@ -0,0 +1,3 @@
>+Tests that decimal numbers typed while holding down the Control key are inserted.
>+
>+PASS
>diff --git a/LayoutTests/fast/events/ios/type-digits-holding-control-key.html b/LayoutTests/fast/events/ios/type-digits-holding-control-key.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..667d51a2ea3f8a02a0bb68c2e7c33bbc1b560348
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/type-digits-holding-control-key.html
>@@ -0,0 +1,53 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<script src="../../../resources/ui-helper.js"></script>
>+<script type="text/plain" id="ui-script">
>+const charactersToType = "1234567890".split("");
>+for (const c of charactersToType)
>+    uiController.keyDown(c, ["ctrlKey"]);
>+</script>
>+<script>
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+}
>+
>+const expectedCharacters = "1234567890";
>+
>+function checkDone()
>+{
>+    let input = document.getElementById("input");
>+    if (input.value === expectedCharacters) {
>+        document.getElementById("result").textContent = "PASS";
>+        document.body.removeChild(input);
>+        if (window.testRunner)
>+            testRunner.notifyDone();
>+    }
>+}
>+
>+function runTest()
>+{
>+    if (!window.testRunner)
>+        return;
>+
>+    function handleFocus() {
>+        testRunner.runUIScript(document.getElementById("ui-script").text, () => { /* Do nothing */ });
>+    }
>+    let input = document.getElementById("input");
>+    input.addEventListener("focus", handleFocus, { once: true });
>+    UIHelper.activateFormControl(input);
>+}
>+</script>
>+</head>
>+<body onload="runTest()">
>+<p>Tests that decimal numbers typed while holding down the <key>Control</key> key are inserted.</p>
>+
>+<input type="text" id="input" size="100" oninput="checkDone()">
>+<div id="result">FAIL</div>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/ios-wk1/TestExpectations b/LayoutTests/platform/ios-wk1/TestExpectations
>index 6cf9f283ec3c0853f85f74332f6d4117bd76a219..8c6bc5df332c501683da1714157daf9433726a32 100644
>--- a/LayoutTests/platform/ios-wk1/TestExpectations
>+++ b/LayoutTests/platform/ios-wk1/TestExpectations
>@@ -33,6 +33,9 @@ imported/w3c/web-platform-tests/html/semantics/forms/the-datalist-element [ Wont
> 
> # No support for testing key commands with modifiers in WK1
> fast/events/ios/focus-tab-previous-field.html [ Skip ]
>+fast/events/ios/key-command-italic-dispatches-keydown.html [ Skip ]
>+fast/events/ios/key-command-italic.html [ Skip ]
>+fast/events/ios/type-digits-holding-control-key.html [ Skip ]
> 
> # <input type=color> is not supported in WebKit1 on iOS.
> fast/forms/color/input-color-onchange-event.html [ Failure ]

Actions: View | Formatted Diff | Diff

Attachments on bug 191608: 354971 | 354992 | 356254 | 356326 | 356394