A focused node should not be assisted when handling touch events synchronously
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Aug 2015 21:48:50 +0000 (21:48 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Aug 2015 21:48:50 +0000 (21:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147836
.:

Reviewed by Enrica Casucci.

Added manual tests for keyboard assistance behavior due to receiving touch events on iOS.

* ManualTests/ios/focused-input-should-assist-on-touch.html: Checks that a currently focused
        input can still be assisted due to a touch event.
* ManualTests/ios/keyboard-should-not-show-on-touch-event.html: Checks that handling a touch
        event does not automatically cause us to assist the currently focused node.

Source/WebCore:

<rdar://problem/22204108>

Reviewed by Enrica Casucci.

Makes interaction with touch handlers no longer assist the currently focused element in the
general case. Added plumbing to reassist a currently focused node when dispatching touch events,
so that an input that programmatically focuses itself and prevents default on a touch event will
be properly assisted when it has been programmatically focused (either through Javascript or the
autofocus attribute) prior to receiving the touch event. This patch also removes the now
unnecessary special-casing of the Gmail settings app that currently makes the keyboard deploy
upon autofocus.

* dom/Element.cpp:
(WebCore::Element::focus): Notifies the chrome client that the element has refocused before
    returning early.
* page/ChromeClient.h: Refocusing an element does nothing by default.
* platform/RuntimeApplicationChecksIOS.h: Removed special casing for Gmail Add Account.
* platform/RuntimeApplicationChecksIOS.mm: See above.
(WebCore::applicationIsGmailAddAccountOnIOS): See above.

Source/WebKit2:

<rdar://problem/22204108>

Reviewed by Enrica Casucci.

Makes interaction with touch handlers no longer assist the currently focused element in the
general case. Added plumbing to reassist a currently focused node when dispatching touch events,
so that an input that programmatically focuses itself and prevents default on a touch event will
be properly assisted when it has been programmatically focused (either through Javascript or the
autofocus attribute) prior to receiving the touch event. This patch also removes the now
unnecessary special-casing of the Gmail settings app that currently makes the keyboard deploy
upon autofocus.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:userObject:]): Removed
    special case to avoid the early return for Gmail Add Account.
* WebProcess/WebCoreSupport/WebChromeClient.h: Added a handler for refocusing an element.
* WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm:
(WebKit::WebChromeClient::elementDidRefocus): Makes refocusing an element trigger input
    assistance on iOS.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::dispatchTouchEvent): Removes logic to focus the currently focused element upon
    receiving a touch event.

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

13 files changed:
ChangeLog
ManualTests/ios/focused-input-should-assist-on-touch.html [new file with mode: 0644]
ManualTests/ios/keyboard-should-not-show-on-touch-event.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/page/ChromeClient.h
Source/WebCore/platform/RuntimeApplicationChecksIOS.h
Source/WebCore/platform/RuntimeApplicationChecksIOS.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit2/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm
Source/WebKit2/WebProcess/WebPage/WebPage.cpp

index 828b4d7..00dbad5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2015-08-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        A focused node should not be assisted when handling touch events synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=147836
+
+        Reviewed by Enrica Casucci.
+
+        Added manual tests for keyboard assistance behavior due to receiving touch events on iOS.
+
+        * ManualTests/ios/focused-input-should-assist-on-touch.html: Checks that a currently focused
+                input can still be assisted due to a touch event.
+        * ManualTests/ios/keyboard-should-not-show-on-touch-event.html: Checks that handling a touch
+                event does not automatically cause us to assist the currently focused node.
+
 2015-08-12  Alex Christensen  <achristensen@webkit.org>
 
         Fix Debug CMake builds on Windows
diff --git a/ManualTests/ios/focused-input-should-assist-on-touch.html b/ManualTests/ios/focused-input-should-assist-on-touch.html
new file mode 100644 (file)
index 0000000..4238cf3
--- /dev/null
@@ -0,0 +1,25 @@
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script>
+        function handleTouchEvent(event) {
+            event.target.focus();
+            event.preventDefault();
+        }
+    </script>
+
+    <style>
+        input:focus {
+            outline: none;
+            border: 1px solid #4D90FE;
+        }
+    </style>
+
+</head>
+
+<body style="margin: 0;">
+    <p>This tests checks that a node can be assisted when focused due to a touch event, even when the focus does not change.</p>
+    <p>Tapping the below input should show the keyboard.</p>
+    <input autofocus ontouchstart="handleTouchEvent(event);"></input>
+</body>
+</html>
diff --git a/ManualTests/ios/keyboard-should-not-show-on-touch-event.html b/ManualTests/ios/keyboard-should-not-show-on-touch-event.html
new file mode 100644 (file)
index 0000000..e2fa770
--- /dev/null
@@ -0,0 +1,34 @@
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script>
+        function touchTest(event) { }
+    </script>
+
+    <style>
+        button {
+            top: 300px;
+            left: 100px;
+            width: 200px;
+            height: 200px;
+            background-color: #FFAAAA;
+            position: absolute;
+            padding: 10px;
+        }
+
+        input:focus {
+            outline: none;
+            border: 1px solid #4D90FE;
+        }
+    </style>
+
+</head>
+
+<body style="margin: 0;">
+    <button ontouchstart="touchTest(event);">
+        <p>This test checks that any interaction that fires touch events will not assist the currently focused node.</p>
+        <p>Thus, tapping in this touch-handling region should not deploy the keyboard for the autofocused input above.</p>
+    </button>
+    <input autofocus></input>
+</body>
+</html>
index 5c6c0f5..7efd1d9 100644 (file)
@@ -1,3 +1,27 @@
+2015-08-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        A focused node should not be assisted when handling touch events synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=147836
+        <rdar://problem/22204108>
+
+        Reviewed by Enrica Casucci.
+
+        Makes interaction with touch handlers no longer assist the currently focused element in the
+        general case. Added plumbing to reassist a currently focused node when dispatching touch events,
+        so that an input that programmatically focuses itself and prevents default on a touch event will
+        be properly assisted when it has been programmatically focused (either through Javascript or the
+        autofocus attribute) prior to receiving the touch event. This patch also removes the now
+        unnecessary special-casing of the Gmail settings app that currently makes the keyboard deploy
+        upon autofocus.
+
+        * dom/Element.cpp:
+        (WebCore::Element::focus): Notifies the chrome client that the element has refocused before
+            returning early.
+        * page/ChromeClient.h: Refocusing an element does nothing by default.
+        * platform/RuntimeApplicationChecksIOS.h: Removed special casing for Gmail Add Account.
+        * platform/RuntimeApplicationChecksIOS.mm: See above.
+        (WebCore::applicationIsGmailAddAccountOnIOS): See above.
+
 2015-08-13  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Unreviewed build fix.
index dbd9097..0c95df7 100644 (file)
@@ -2117,8 +2117,12 @@ void Element::focus(bool restorePreviousSelection, FocusDirection direction)
     if (!inDocument())
         return;
 
-    if (document().focusedElement() == this)
+    if (document().focusedElement() == this) {
+        if (document().page())
+            document().page()->chrome().client().elementDidRefocus(this);
+
         return;
+    }
 
     // If the stylesheets have already been loaded we can reliably check isFocusable.
     // If not, we continue and set the focused node on the focus controller below so
index ee0fb48..f6a8ede 100644 (file)
@@ -281,6 +281,7 @@ public:
         
     virtual void elementDidFocus(const Node*) { };
     virtual void elementDidBlur(const Node*) { };
+    virtual void elementDidRefocus(const Node*) { };
     
     virtual bool shouldPaintEntireContents() const { return false; }
     virtual bool hasStablePageScaleFactor() const { return true; }
index 691c8cb..77a8da9 100644 (file)
@@ -43,7 +43,6 @@ bool applicationIsMASH();
 WEBCORE_EXPORT bool applicationIsTheEconomistOnIPhone();
 bool applicationIsWebProcess();
 bool applicationIsIBooksOnIOS();
-WEBCORE_EXPORT bool applicationIsGmailAddAccountOnIOS();
 
 } // namespace WebCore
 
index 1a541bb..4629f44 100644 (file)
@@ -122,12 +122,6 @@ bool applicationIsIBooksOnIOS()
     return isIBooksOnIOS;
 }
 
-bool applicationIsGmailAddAccountOnIOS()
-{
-    static const bool isGmailAddAccountOnIOS = [[[NSBundle mainBundle] bundleIdentifier] isEqualToString:@"com.apple.social.SLGoogleAuth.SLGoogleAuthService"];
-    return isGmailAddAccountOnIOS;
-}
-
 } // namespace WebCore
 
 #endif // PLATFORM(IOS)
index 8b7867a..dde6fbb 100644 (file)
@@ -1,3 +1,30 @@
+2015-08-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        A focused node should not be assisted when handling touch events synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=147836
+        <rdar://problem/22204108>
+
+        Reviewed by Enrica Casucci.
+
+        Makes interaction with touch handlers no longer assist the currently focused element in the
+        general case. Added plumbing to reassist a currently focused node when dispatching touch events,
+        so that an input that programmatically focuses itself and prevents default on a touch event will
+        be properly assisted when it has been programmatically focused (either through Javascript or the
+        autofocus attribute) prior to receiving the touch event. This patch also removes the now
+        unnecessary special-casing of the Gmail settings app that currently makes the keyboard deploy
+        upon autofocus.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:userObject:]): Removed
+            special case to avoid the early return for Gmail Add Account.
+        * WebProcess/WebCoreSupport/WebChromeClient.h: Added a handler for refocusing an element.
+        * WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm:
+        (WebKit::WebChromeClient::elementDidRefocus): Makes refocusing an element trigger input
+            assistance on iOS.
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::dispatchTouchEvent): Removes logic to focus the currently focused element upon
+            receiving a touch event.
+
 2015-08-13  Anders Carlsson  <andersca@apple.com>
 
         Add WKWindowFeaturesRef and a new modern createNewPage UI client callback
index f93807c..83e6ba2 100644 (file)
@@ -3012,7 +3012,7 @@ static bool isAssistableInputType(InputType type)
 {
     // FIXME: This is a temporary workaround for <rdar://problem/22126518>. The real fix will involve refactoring
     // the way we assist programmatically focused nodes.
-    if (!applicationIsGmailAddAccountOnIOS() && !userIsInteracting && !_textSelectionAssistant)
+    if (!userIsInteracting && !_textSelectionAssistant)
         return;
 
     if (blurPreviousNode)
index c1ed387..20952d1 100644 (file)
@@ -243,6 +243,7 @@ private:
 #if PLATFORM(IOS)
     virtual void elementDidFocus(const WebCore::Node*) override;
     virtual void elementDidBlur(const WebCore::Node*) override;
+    virtual void elementDidRefocus(const WebCore::Node*) override;
 #endif
 
 #if PLATFORM(IOS)
index a25591e..c2421c9 100644 (file)
@@ -54,6 +54,11 @@ void WebChromeClient::elementDidBlur(const WebCore::Node* node)
     m_page->elementDidBlur(const_cast<WebCore::Node*>(node));
 }
 
+void WebChromeClient::elementDidRefocus(const WebCore::Node* node)
+{
+    elementDidFocus(node);
+}
+
 void WebChromeClient::didReceiveMobileDocType(bool isMobileDoctype)
 {
     m_page->didReceiveMobileDocType(isMobileDoctype);
index db77d10..e2798c1 100644 (file)
@@ -2190,24 +2190,12 @@ static bool handleTouchEvent(const WebTouchEvent& touchEvent, Page* page)
 #if ENABLE(IOS_TOUCH_EVENTS)
 void WebPage::dispatchTouchEvent(const WebTouchEvent& touchEvent, bool& handled)
 {
-    RefPtr<Frame> oldFocusedFrame = m_page->focusController().focusedFrame();
-    RefPtr<Element> oldFocusedElement = oldFocusedFrame ? oldFocusedFrame->document()->focusedElement() : nullptr;
     m_userIsInteracting = true;
 
     m_lastInteractionLocation = touchEvent.position();
     CurrentEvent currentEvent(touchEvent);
     handled = handleTouchEvent(touchEvent, m_page.get());
 
-    RefPtr<Frame> newFocusedFrame = m_page->focusController().focusedFrame();
-    RefPtr<Element> newFocusedElement = newFocusedFrame ? newFocusedFrame->document()->focusedElement() : nullptr;
-
-    // If the focus has not changed, we need to notify the client anyway, since it might be
-    // necessary to start assisting the node.
-    // If the node has been focused by JavaScript without user interaction, the
-    // keyboard is not on screen.
-    if (newFocusedElement && newFocusedElement == oldFocusedElement)
-        elementDidFocus(newFocusedElement.get());
-
     m_userIsInteracting = false;
 }