Touching media controls sometimes shows software keyboard
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jul 2019 16:22:42 +0000 (16:22 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jul 2019 16:22:42 +0000 (16:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199490
<rdar://problem/52076270>

Reviewed by Eric Carlson.

Source/WebKit:

In r243044, we added a compatibility hack for Google Slides (and other G-suite properties) to allow the on-
screen keyboard to show up after a prevented touch event in the case where an element was already focused, even
if the touch event handler doesn't explicitly refocus the element. However, this means that if a regular text
field (or other form control) has been programmatically focused, then interacting with any other element that
prevents default on touchstart will cause us to show the keyboard for that focused element.

To mitigate this, only fall down this refocusing codepath in the case where the focused element is a hidden
editable element (in the style of many Google productivity web apps). For non-hidden editable elements that are
already focused, this refocusing logic is not necessary, since the user should be able to interact with the
control to show the keyboard anyways; for hidden editable areas, this compatibility hack is actually needed,
since there is typically no other way for a user to focus these elements and show an on-screen keyboard.

Tests:  fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html
        fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::dispatchTouchEvent):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::isTransparentOrFullyClipped const):

Renamed from enclosingLayerIsTransparentOrFullyClipped, and pulled out into a private helper method.

(WebKit::WebPage::platformEditorState const):
(WebKit::WebPage::requestEvasionRectsAboveSelection):
(WebKit::WebPage::getFocusedElementInformation):
(WebKit::enclosingLayerIsTransparentOrFullyClipped): Deleted.

Tools:

Adds plumbing for a new testing hook to check whether or not there is an active input session. See other
ChangeLog entries for more detail.

* DumpRenderTree/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::hasInputSession const):
* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* TestRunnerShared/UIScriptContext/UIScriptController.cpp:
(WTR::UIScriptController::hasInputSession const):
* TestRunnerShared/UIScriptContext/UIScriptController.h:
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::hasInputSession const):

LayoutTests:

Adds a new layout test to verify that the keyboard only appears after a handled touch event if the focused
element is inside a hidden editable area; otherwise, the keyboard should not be present.

* fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart-expected.txt: Added.
* fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html: Added.

This test passes as long as we didn't begin showing the keyboard after tapping.

* fast/events/touch/ios/show-keyboard-after-preventing-touchstart-expected.txt:
* fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html:

Adjust this existing test to make the focused textarea hidden.

* resources/ui-helper.js:
(window.UIHelper.hasInputSession):

Add a new testing hook to check whether there is an active input session.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247158 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/show-keyboard-after-preventing-touchstart-expected.txt
LayoutTests/fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html
LayoutTests/resources/ui-helper.js
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

index 2a0e142..4396935 100644 (file)
@@ -1,3 +1,29 @@
+2019-07-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Touching media controls sometimes shows software keyboard
+        https://bugs.webkit.org/show_bug.cgi?id=199490
+        <rdar://problem/52076270>
+
+        Reviewed by Eric Carlson.
+
+        Adds a new layout test to verify that the keyboard only appears after a handled touch event if the focused
+        element is inside a hidden editable area; otherwise, the keyboard should not be present.
+
+        * fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart-expected.txt: Added.
+        * fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html: Added.
+
+        This test passes as long as we didn't begin showing the keyboard after tapping.
+
+        * fast/events/touch/ios/show-keyboard-after-preventing-touchstart-expected.txt:
+        * fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html:
+
+        Adjust this existing test to make the focused textarea hidden.
+
+        * resources/ui-helper.js:
+        (window.UIHelper.hasInputSession):
+
+        Add a new testing hook to check whether there is an active input session.
+
 2019-07-05  Antoine Quint  <graouts@apple.com>
 
         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
diff --git a/LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart-expected.txt b/LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart-expected.txt
new file mode 100644 (file)
index 0000000..8dc7ff7
--- /dev/null
@@ -0,0 +1,11 @@
+
+Verifies that the keyboard does not show up even after preventing default on touchstart when a regular input field is focused. To manually test, tap the red box; the keyboard should not appear.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS hasInputSession is false
+PASS document.activeElement is textarea
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html b/LayoutTests/fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html
new file mode 100644 (file)
index 0000000..9225fb3
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../../resources/js-test.js"></script>
+<script src="../../../../resources/ui-helper.js"></script>
+<style>
+html, body {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+
+textarea, #target {
+    width: 100%;
+    height: 100px;
+}
+
+#target {
+    background-color: tomato;
+    opacity: 0.25;
+}
+</style>
+</head>
+<body>
+    <textarea></textarea>
+    <div id="target"></div>
+    <pre id="description"></pre>
+    <pre id="console"></pre>
+</body>
+<script>
+    jsTestIsAsync = true;
+    textarea = document.querySelector("textarea");
+    target = document.getElementById("target");
+    target.addEventListener("touchstart", event => event.preventDefault());
+
+    description("Verifies that the keyboard does not show up even after preventing default on touchstart when a regular input field is focused. To manually test, tap the red box; the keyboard should not appear.");
+
+    addEventListener("load", async () => {
+        textarea.focus();
+        await UIHelper.activateElement(target);
+        await UIHelper.ensurePresentationUpdate();
+        hasInputSession = await UIHelper.hasInputSession();
+        shouldBe("hasInputSession", "false");
+        shouldBe("document.activeElement", "textarea");
+        finishJSTest();
+    });
+</script>
+</html>
index d8c45bf..02f7935 100644 (file)
@@ -1,5 +1,5 @@
 
-Verifies that the keyboard shows up even after preventing default on touchstart. To manually test, tap the textarea; the textarea should remain focused, and the keyboard should appear.
+Verifies that the keyboard shows up even after preventing default on touchstart when focusing a hidden editable area. To manually test, tap the red box; the textarea should remain focused, and the keyboard should appear.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
index b25af03..48a6ad7 100644 (file)
@@ -10,29 +10,45 @@ html, body {
     margin: 0;
 }
 
-textarea {
+textarea, #target {
     width: 100%;
     height: 100px;
-    font-size: 50px;
-    text-align: center;
+    position: absolute;
+    top: 0;
+}
+
+#target {
+    background-color: tomato;
+    z-index: 1;
+    opacity: 0.25;
+}
+
+textarea {
+    opacity: 0;
+}
+
+#description {
+    margin-top: 100px;
 }
 </style>
 </head>
 <body>
     <textarea></textarea>
+    <div id="target"></div>
     <pre id="description"></pre>
     <pre id="console"></pre>
 </body>
 <script>
     jsTestIsAsync = true;
     textarea = document.querySelector("textarea");
+    target = document.getElementById("target");
+    target.addEventListener("touchstart", event => event.preventDefault());
 
-    description("Verifies that the keyboard shows up even after preventing default on touchstart. To manually test, tap the textarea; the textarea should remain focused, and the keyboard should appear.");
+    description("Verifies that the keyboard shows up even after preventing default on touchstart when focusing a hidden editable area. To manually test, tap the red box; the textarea should remain focused, and the keyboard should appear.");
 
-    textarea.addEventListener("touchstart", event => event.preventDefault());
     addEventListener("load", async () => {
         textarea.focus();
-        await UIHelper.activateElementAndWaitForInputSession(textarea);
+        await UIHelper.activateElementAndWaitForInputSession(target);
         testPassed("keyboard was shown.");
         shouldBe("document.activeElement", "textarea");
         textarea.blur();
index 9eb97d0..96684aa 100644 (file)
@@ -372,6 +372,13 @@ window.UIHelper = class UIHelper {
         });
     }
 
+    static hasInputSession()
+    {
+        return new Promise(resolve => {
+            testRunner.runUIScript("uiController.hasInputSession", result => resolve(result === "true"));
+        });
+    }
+
     static isPresentingModally()
     {
         return new Promise(resolve => {
index 9a39854..9205cb7 100644 (file)
@@ -1,3 +1,39 @@
+2019-07-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Touching media controls sometimes shows software keyboard
+        https://bugs.webkit.org/show_bug.cgi?id=199490
+        <rdar://problem/52076270>
+
+        Reviewed by Eric Carlson.
+
+        In r243044, we added a compatibility hack for Google Slides (and other G-suite properties) to allow the on-
+        screen keyboard to show up after a prevented touch event in the case where an element was already focused, even
+        if the touch event handler doesn't explicitly refocus the element. However, this means that if a regular text
+        field (or other form control) has been programmatically focused, then interacting with any other element that
+        prevents default on touchstart will cause us to show the keyboard for that focused element.
+
+        To mitigate this, only fall down this refocusing codepath in the case where the focused element is a hidden
+        editable element (in the style of many Google productivity web apps). For non-hidden editable elements that are
+        already focused, this refocusing logic is not necessary, since the user should be able to interact with the
+        control to show the keyboard anyways; for hidden editable areas, this compatibility hack is actually needed,
+        since there is typically no other way for a user to focus these elements and show an on-screen keyboard.
+
+        Tests:  fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html
+                fast/events/touch/ios/do-not-show-keyboard-after-preventing-touchstart.html
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::dispatchTouchEvent):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::isTransparentOrFullyClipped const):
+
+        Renamed from enclosingLayerIsTransparentOrFullyClipped, and pulled out into a private helper method.
+
+        (WebKit::WebPage::platformEditorState const):
+        (WebKit::WebPage::requestEvasionRectsAboveSelection):
+        (WebKit::WebPage::getFocusedElementInformation):
+        (WebKit::enclosingLayerIsTransparentOrFullyClipped): Deleted.
+
 2019-07-04  Chris Dumez  <cdumez@apple.com>
 
         Simplify logic that handles registering WebProcessProxy objects with their WebsiteDataStore
index 3bcf0d7..d7f8c31 100644 (file)
@@ -2896,7 +2896,7 @@ void WebPage::dispatchTouchEvent(const WebTouchEvent& touchEvent, bool& handled)
     if (handled && oldFocusedElement) {
         auto newFocusedFrame = makeRefPtr(m_page->focusController().focusedFrame());
         auto newFocusedElement = makeRefPtr(newFocusedFrame ? newFocusedFrame->document()->focusedElement() : nullptr);
-        if (oldFocusedElement == newFocusedElement)
+        if (oldFocusedElement == newFocusedElement && isTransparentOrFullyClipped(*newFocusedElement))
             elementDidRefocus(*newFocusedElement);
     }
 }
index 7d79a78..a490b09 100644 (file)
@@ -1448,6 +1448,7 @@ private:
 
 #if PLATFORM(IOS_FAMILY)
     void didChooseFilesForOpenPanelWithDisplayStringAndIcon(const Vector<String>&, const String& displayString, const IPC::DataReference& iconData);
+    bool isTransparentOrFullyClipped(const WebCore::Element&) const;
 #endif
 
 #if ENABLE(SANDBOX_EXTENSIONS)
index c31c724..3268402 100644 (file)
@@ -185,9 +185,13 @@ static void computeEditableRootHasContentAndPlainText(const VisibleSelection& se
     data.hasPlainText = data.hasContent && hasAnyPlainText(Range::create(root->document(), VisiblePosition { startInEditableRoot }, VisiblePosition { lastPositionInNode(root) }));
 }
 
-static bool enclosingLayerIsTransparentOrFullyClipped(const RenderObject& renderer)
+bool WebPage::isTransparentOrFullyClipped(const Element& element) const
 {
-    auto* enclosingLayer = renderer.enclosingLayer();
+    auto* renderer = element.renderer();
+    if (!renderer)
+        return false;
+
+    auto* enclosingLayer = renderer->enclosingLayer();
     return enclosingLayer && enclosingLayer->isTransparentOrFullyClippedRespectingParentFrames();
 }
 
@@ -264,9 +268,8 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
             postLayoutData.caretColor = renderer.style().caretColor();
         }
         if (result.isContentEditable) {
-            auto container = makeRefPtr(selection.rootEditableElement());
-            if (container && container->renderer())
-                postLayoutData.editableRootIsTransparentOrFullyClipped = enclosingLayerIsTransparentOrFullyClipped(*container->renderer());
+            if (auto container = makeRefPtr(selection.rootEditableElement()))
+                postLayoutData.editableRootIsTransparentOrFullyClipped = isTransparentOrFullyClipped(*container);
         }
         computeEditableRootHasContentAndPlainText(selection, postLayoutData);
     }
@@ -1801,7 +1804,7 @@ void WebPage::requestEvasionRectsAboveSelection(CompletionHandler<void(const Vec
         return;
     }
 
-    if (!m_focusedElement || !m_focusedElement->renderer() || enclosingLayerIsTransparentOrFullyClipped(*m_focusedElement->renderer())) {
+    if (!m_focusedElement || !m_focusedElement->renderer() || isTransparentOrFullyClipped(*m_focusedElement)) {
         reply({ });
         return;
     }
@@ -2991,7 +2994,7 @@ void WebPage::getFocusedElementInformation(FocusedElementInformation& informatio
         information.isReadOnly = false;
     }
 
-    if (m_focusedElement->document().quirks().shouldSuppressAutocorrectionAndAutocaptializationInHiddenEditableAreas() && m_focusedElement->renderer() && enclosingLayerIsTransparentOrFullyClipped(*m_focusedElement->renderer())) {
+    if (m_focusedElement->document().quirks().shouldSuppressAutocorrectionAndAutocaptializationInHiddenEditableAreas() && isTransparentOrFullyClipped(*m_focusedElement)) {
         information.autocapitalizeType = AutocapitalizeTypeNone;
         information.isAutocorrect = false;
     }
index dc9ec41..138c46e 100644 (file)
@@ -1,3 +1,23 @@
+2019-07-05  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Touching media controls sometimes shows software keyboard
+        https://bugs.webkit.org/show_bug.cgi?id=199490
+        <rdar://problem/52076270>
+
+        Reviewed by Eric Carlson.
+
+        Adds plumbing for a new testing hook to check whether or not there is an active input session. See other
+        ChangeLog entries for more detail.
+
+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::hasInputSession const):
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::hasInputSession const):
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::hasInputSession const):
+
 2019-07-04  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-build] Remove GTK and WPE queue from old EWS and dashboard
index ba1f3ed..d7ea466 100644 (file)
@@ -287,6 +287,11 @@ bool UIScriptController::isShowingKeyboard() const
     return false;
 }
 
+bool UIScriptController::hasInputSession() const
+{
+    return false;
+}
+
 double UIScriptController::minimumZoomScale() const
 {
     return gWebScrollView.minimumZoomScale;
index 3b69cde..9e868ce 100644 (file)
@@ -224,6 +224,7 @@ interface UIScriptController {
     attribute object didShowKeyboardCallback;
     attribute object didHideKeyboardCallback;
     readonly attribute boolean isShowingKeyboard;
+    readonly attribute boolean hasInputSession;
 
     attribute object didShowMenuCallback;
     attribute object didHideMenuCallback;
index 1bedd7e..0f8db13 100644 (file)
@@ -453,6 +453,11 @@ bool UIScriptController::isShowingKeyboard() const
     return false;
 }
 
+bool UIScriptController::hasInputSession() const
+{
+    return false;
+}
+
 double UIScriptController::zoomScale() const
 {
     return 1;
index 0bedc04..18e18ab 100644 (file)
@@ -163,6 +163,7 @@ public:
     JSValueRef didHideKeyboardCallback() const;
 
     bool isShowingKeyboard() const;
+    bool hasInputSession() const;
 
     void setDidHideMenuCallback(JSValueRef);
     JSValueRef didHideMenuCallback() const;
index 9933150..955f654 100644 (file)
@@ -625,6 +625,11 @@ bool UIScriptController::isShowingKeyboard() const
     return TestController::singleton().mainWebView()->platformView().showingKeyboard;
 }
 
+bool UIScriptController::hasInputSession() const
+{
+    return TestController::singleton().mainWebView()->platformView().isInteractingWithFormControl;
+}
+
 void UIScriptController::applyAutocorrection(JSStringRef newString, JSStringRef oldString, JSValueRef callback)
 {
     unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);