iOS: Pressing tab in the Mail subject field moves focus to the body, but pressing...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 11:15:00 +0000 (11:15 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 11:15:00 +0000 (11:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212243
<rdar://problem/59127764>

Reviewed by Wenson Hsieh.

Source/WebCore:

New API Tests: WebKit.ShiftTabTakesFocusFromEditableWebView and WebKit.TabDoesNotTakeFocusFromEditableWebView

* page/FocusController.cpp:
(WebCore::FocusController::relinquishFocusToChrome):
(WebCore::FocusController::advanceFocusInDocumentOrder):
* page/FocusController.h:
Factor out the code that decides whether the Chrome might accept focus,
and transfers focus out to the Chrome, for use in EventHandler.

* page/EventHandler.cpp:
(WebCore::EventHandler::defaultTabEventHandler):
In the case where we are shift-tabbing out of an editable web view,
allow focus to pass to the Chrome. Previously, we would not allow this,
because tabKeyCyclesThroughElements is false in editable web views.
However, focus exiting the web view entirely needn't be covered by
"cycles through elements" behavior.
We can't do this for plain "tab", because that needs to be allowed to
insert a tab character instead.

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:
(-[FocusDelegate _webView:takeFocus:]):
(-[FocusDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
(TEST):

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

Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/FocusController.cpp
Source/WebCore/page/FocusController.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm

index dab6586..dfd3ebf 100644 (file)
@@ -1,3 +1,30 @@
+2020-05-22  Tim Horton  <timothy_horton@apple.com>
+
+        iOS: Pressing tab in the Mail subject field moves focus to the body, but pressing shift tab doesn't move it back
+        https://bugs.webkit.org/show_bug.cgi?id=212243
+        <rdar://problem/59127764>
+
+        Reviewed by Wenson Hsieh.
+
+        New API Tests: WebKit.ShiftTabTakesFocusFromEditableWebView and WebKit.TabDoesNotTakeFocusFromEditableWebView
+
+        * page/FocusController.cpp:
+        (WebCore::FocusController::relinquishFocusToChrome):
+        (WebCore::FocusController::advanceFocusInDocumentOrder):
+        * page/FocusController.h:
+        Factor out the code that decides whether the Chrome might accept focus,
+        and transfers focus out to the Chrome, for use in EventHandler.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::defaultTabEventHandler):
+        In the case where we are shift-tabbing out of an editable web view,
+        allow focus to pass to the Chrome. Previously, we would not allow this,
+        because tabKeyCyclesThroughElements is false in editable web views.
+        However, focus exiting the web view entirely needn't be covered by
+        "cycles through elements" behavior.
+        We can't do this for plain "tab", because that needs to be allowed to
+        insert a tab character instead.
+
 2020-05-22  Tyler Wilcock  <twilco.o@protonmail.com>
 
         Cannot style ::selection for a flex container
index 55bd026..4c6fb95 100644 (file)
@@ -4094,8 +4094,6 @@ void EventHandler::defaultTabEventHandler(KeyboardEvent& event)
     Page* page = m_frame.page();
     if (!page)
         return;
-    if (!page->tabKeyCyclesThroughElements())
-        return;
 
     FocusDirection focusDirection = event.shiftKey() ? FocusDirectionBackward : FocusDirectionForward;
 
@@ -4103,6 +4101,17 @@ void EventHandler::defaultTabEventHandler(KeyboardEvent& event)
     if (m_frame.document()->inDesignMode())
         return;
 
+    // Allow shift-tab to relinquish focus even if we don't allow tab to cycle between elements inside the view.
+    // We can only do this for shift-tab, not tab itself because tabKeyCyclesThroughElements is used to make
+    // tab character insertion work in editable web views.
+    if (!page->tabKeyCyclesThroughElements()) {
+        if (focusDirection == FocusDirectionBackward) {
+            if (page->focusController().relinquishFocusToChrome(focusDirection))
+                event.setDefaultHandled();
+        }
+        return;
+    }
+
     if (page->focusController().advanceFocus(focusDirection, &event))
         event.setDefaultHandled();
 }
index ff413d9..6785472 100644 (file)
@@ -448,6 +448,21 @@ bool FocusController::advanceFocus(FocusDirection direction, KeyboardEvent* even
     return false;
 }
 
+bool FocusController::relinquishFocusToChrome(FocusDirection direction)
+{
+    RefPtr<Document> document = focusedOrMainFrame().document();
+    if (!document)
+        return false;
+
+    if (!m_page.chrome().canTakeFocus(direction) || m_page.isControlledByAutomation())
+        return false;
+
+    document->setFocusedElement(nullptr);
+    setFocusedFrame(nullptr);
+    m_page.chrome().takeFocus(direction);
+    return true;
+}
+
 bool FocusController::advanceFocusInDocumentOrder(FocusDirection direction, KeyboardEvent* event, bool initialFocus)
 {
     Frame& frame = focusedOrMainFrame();
@@ -466,11 +481,9 @@ bool FocusController::advanceFocusInDocumentOrder(FocusDirection direction, Keyb
 
     if (!element) {
         // We didn't find a node to focus, so we should try to pass focus to Chrome.
-        if (!initialFocus && m_page.chrome().canTakeFocus(direction) && !m_page.isControlledByAutomation()) {
-            document->setFocusedElement(nullptr);
-            setFocusedFrame(nullptr);
-            m_page.chrome().takeFocus(direction);
-            return true;
+        if (!initialFocus) {
+            if (relinquishFocusToChrome(direction))
+                return true;
         }
 
         // Chrome doesn't want focus, so we should wrap focus.
index 6cd600f..17b725a 100644 (file)
@@ -78,6 +78,8 @@ public:
     void setFocusedElementNeedsRepaint();
     Seconds timeSinceFocusWasSet() const;
 
+    bool relinquishFocusToChrome(FocusDirection);
+
 private:
     void setActiveInternal(bool);
     void setFocusedInternal(bool);
index c073346..f56a5b7 100644 (file)
@@ -1,3 +1,16 @@
+2020-05-22  Tim Horton  <timothy_horton@apple.com>
+
+        iOS: Pressing tab in the Mail subject field moves focus to the body, but pressing shift tab doesn't move it back
+        https://bugs.webkit.org/show_bug.cgi?id=212243
+        <rdar://problem/59127764>
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:
+        (-[FocusDelegate _webView:takeFocus:]):
+        (-[FocusDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+        (TEST):
+
 2020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         DataTransfer.files contains multiple files when pasting a single image with multiple representations
index 2af9985..229ee9b 100644 (file)
@@ -809,23 +809,28 @@ static void synthesizeTab(NSWindow *window, NSView *view, bool withShiftDown)
     [view keyUp:tabEvent(window, NSEventTypeKeyUp, withShiftDown ? NSEventModifierFlagShift : 0)];
 }
 
-static _WKFocusDirection takenDirection;
-
 @interface FocusDelegate : NSObject <WKUIDelegatePrivate>
+@property (nonatomic) _WKFocusDirection takenDirection;
+@property (nonatomic) BOOL useShiftTab;
 @end
 
-@implementation FocusDelegate
+@implementation FocusDelegate {
+@package
+    bool _done;
+    bool _didSendKeyEvent;
+}
 
 - (void)_webView:(WKWebView *)webView takeFocus:(_WKFocusDirection)direction
 {
-    takenDirection = direction;
-    done = true;
+    _takenDirection = direction;
+    _done = true;
 }
 
 - (void)webView:(WKWebView *)webView runJavaScriptAlertPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(void))completionHandler
 {
     completionHandler();
-    synthesizeTab([webView window], webView, true);
+    _didSendKeyEvent = true;
+    synthesizeTab([webView window], webView, _useShiftTab);
 }
 
 @end
@@ -834,12 +839,43 @@ TEST(WebKit, Focus)
 {
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
     auto delegate = adoptNS([[FocusDelegate alloc] init]);
+    [delegate setUseShiftTab:YES];
     [webView setUIDelegate:delegate.get()];
     NSString *html = @"<script>function loaded() { document.getElementById('in').focus(); alert('ready'); }</script>"
     "<body onload='loaded()'><input type='text' id='in'></body>";
     [webView loadHTMLString:html baseURL:[NSURL URLWithString:@"http://example.com/"]];
-    TestWebKitAPI::Util::run(&done);
-    ASSERT_EQ(takenDirection, _WKFocusDirectionBackward);
+    TestWebKitAPI::Util::run(&delegate->_done);
+    ASSERT_EQ([delegate takenDirection], _WKFocusDirectionBackward);
+}
+
+TEST(WebKit, ShiftTabTakesFocusFromEditableWebView)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
+    [webView _setEditable:YES];
+
+    auto delegate = adoptNS([[FocusDelegate alloc] init]);
+    [delegate setUseShiftTab:YES];
+    [webView setUIDelegate:delegate.get()];
+    NSString *html = @"<script>function loaded() { document.body.focus(); alert('ready'); }</script>"
+    "<body onload='loaded()'></body>";
+    [webView loadHTMLString:html baseURL:[NSURL URLWithString:@"http://example.com/"]];
+    TestWebKitAPI::Util::run(&delegate->_done);
+    ASSERT_EQ([delegate takenDirection], _WKFocusDirectionBackward);
+}
+
+TEST(WebKit, TabDoesNotTakeFocusFromEditableWebView)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600)]);
+    [webView _setEditable:YES];
+
+    auto delegate = adoptNS([[FocusDelegate alloc] init]);
+    [webView setUIDelegate:delegate.get()];
+    NSString *html = @"<script>function loaded() { document.body.focus(); alert('ready'); }</script>"
+    "<body onload='loaded()'></body>";
+    [webView loadHTMLString:html baseURL:[NSURL URLWithString:@"http://example.com/"]];
+    TestWebKitAPI::Util::run(&delegate->_didSendKeyEvent);
+    EXPECT_WK_STREQ("\t", [webView stringByEvaluatingJavaScript:@"document.body.textContent"]);
+    ASSERT_FALSE(delegate->_done);
 }
 
 #define MOUSE_EVENT_CAUSES_DOWNLOAD 0