[webkit-changes] [238728] trunk

Thu, 29 Nov 2018 23:23:50 -0800

Title: [238728] trunk
Revision
238728
Author
wenson_hs...@apple.com
Date
2018-11-29 23:22:00 -0800 (Thu, 29 Nov 2018)

Log Message

REGRESSION (r238635): Dragging a text selection within WKWebView causes the selection highlight to get into a bad state
https://bugs.webkit.org/show_bug.cgi?id=192165
<rdar://problem/46346682>

Reviewed by Daniel Bates.

Source/WebKit:

Fixes a bug in PageClientImpl::isViewFocused. Consider the following scenario:
1. WKWebView is hosted within the view hierarchy
2. First responder is *not* WKContentView
3. The active focus retain count is nonzero

Before r238635, we would return true, due to condition (3). However, after r238635, we only consider whether the
first responder is WKContentView, since the web view is in the view hierarchy. This breaks scenarios where
WebKit or UIKit attempts to retain focus and later restore the content view to be the first responder (an
example of this is dragging a text selection between editable elements in the same web view).

To fix this, simply bail early and return true if focus is being retained.

* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::isViewFocused):

Tools:

Fixes 11 API tests that started failing or timing out after r238635. See below for more details.

* TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm:
(TestWebKitAPI::webViewForEditActionTesting):
(TestWebKitAPI::webViewForEditActionTestingWithPageNamed):

Ensure that the web view becomes first responder before executing edit actions.

* TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html:
* TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html:

Tweak these tests to allow selected content to overflow the width of the web view. Without this change,
ContentEditableToContentEditable and ContentEditableToTextarea will sometimes fail because the content causes
the body to scroll horizontally, so we miss the drop destination.

* TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
(loadTestPageAndEnsureInputSession):

Add a new helper to load a test page with a given name, become first responder, and wait until an input session
starts. Use this in various drag and drop tests to reduce code duplication.

* TestWebKitAPI/cocoa/DragAndDropSimulator.h:
* TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
(-[DragAndDropSimulator initWithWebView:]):
(-[DragAndDropSimulator _resetSimulatedState]):
(-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
(-[DragAndDropSimulator _advanceProgress]):

To more accurately emulate UIKit behavior, begin focus preservation when starting a drag, and attempt to clear
the focus preservation token when the drag session ends. This allows us to simulate and test the scenario that
regressed with r238635.

(-[DragAndDropSimulator ensureInputSession]):
(-[DragAndDropSimulator _webView:didStartInputSession:]):
(-[DragAndDropSimulator waitForInputSession]): Deleted.

Refactored into -ensureInputSession. Instead of assuming that an input session has not yet been started, simply
wait for an input session to start if needed.

* TestWebKitAPI/ios/UIKitSPI.h:

Add a new SPI declaration.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (238727 => 238728)


--- trunk/Source/WebKit/ChangeLog	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Source/WebKit/ChangeLog	2018-11-30 07:22:00 UTC (rev 238728)
@@ -1,3 +1,26 @@
+2018-11-29  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        REGRESSION (r238635): Dragging a text selection within WKWebView causes the selection highlight to get into a bad state
+        https://bugs.webkit.org/show_bug.cgi?id=192165
+        <rdar://problem/46346682>
+
+        Reviewed by Daniel Bates.
+
+        Fixes a bug in PageClientImpl::isViewFocused. Consider the following scenario:
+        1. WKWebView is hosted within the view hierarchy
+        2. First responder is *not* WKContentView
+        3. The active focus retain count is nonzero
+
+        Before r238635, we would return true, due to condition (3). However, after r238635, we only consider whether the
+        first responder is WKContentView, since the web view is in the view hierarchy. This breaks scenarios where
+        WebKit or UIKit attempts to retain focus and later restore the content view to be the first responder (an
+        example of this is dragging a text selection between editable elements in the same web view).
+
+        To fix this, simply bail early and return true if focus is being retained.
+
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::isViewFocused):
+
 2018-11-29  Tim Horton  <timothy_hor...@apple.com>
 
         Inform clients when editable image attachment backing data changes

Modified: trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm (238727 => 238728)


--- trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm	2018-11-30 07:22:00 UTC (rev 238728)
@@ -166,9 +166,7 @@
 
 bool PageClientImpl::isViewFocused()
 {
-    if (isViewInWindow() && ![m_webView _isBackground])
-        return [m_webView _contentViewIsFirstResponder];
-    return [m_webView _isRetainingActiveFocusedState];
+    return (isViewInWindow() && ![m_webView _isBackground] && [m_webView _contentViewIsFirstResponder]) || [m_webView _isRetainingActiveFocusedState];
 }
 
 bool PageClientImpl::isViewVisible()

Modified: trunk/Tools/ChangeLog (238727 => 238728)


--- trunk/Tools/ChangeLog	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/ChangeLog	2018-11-30 07:22:00 UTC (rev 238728)
@@ -1,3 +1,54 @@
+2018-11-29  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        REGRESSION (r238635): Dragging a text selection within WKWebView causes the selection highlight to get into a bad state
+        https://bugs.webkit.org/show_bug.cgi?id=192165
+        <rdar://problem/46346682>
+
+        Reviewed by Daniel Bates.
+
+        Fixes 11 API tests that started failing or timing out after r238635. See below for more details.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm:
+        (TestWebKitAPI::webViewForEditActionTesting):
+        (TestWebKitAPI::webViewForEditActionTestingWithPageNamed):
+
+        Ensure that the web view becomes first responder before executing edit actions.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html:
+        * TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html:
+
+        Tweak these tests to allow selected content to overflow the width of the web view. Without this change,
+        ContentEditableToContentEditable and ContentEditableToTextarea will sometimes fail because the content causes
+        the body to scroll horizontally, so we miss the drop destination.
+
+        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
+        (loadTestPageAndEnsureInputSession):
+
+        Add a new helper to load a test page with a given name, become first responder, and wait until an input session
+        starts. Use this in various drag and drop tests to reduce code duplication.
+
+        * TestWebKitAPI/cocoa/DragAndDropSimulator.h:
+        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
+        (-[DragAndDropSimulator initWithWebView:]):
+        (-[DragAndDropSimulator _resetSimulatedState]):
+        (-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
+        (-[DragAndDropSimulator _advanceProgress]):
+
+        To more accurately emulate UIKit behavior, begin focus preservation when starting a drag, and attempt to clear
+        the focus preservation token when the drag session ends. This allows us to simulate and test the scenario that
+        regressed with r238635.
+
+        (-[DragAndDropSimulator ensureInputSession]):
+        (-[DragAndDropSimulator _webView:didStartInputSession:]):
+        (-[DragAndDropSimulator waitForInputSession]): Deleted.
+
+        Refactored into -ensureInputSession. Instead of assuming that an input session has not yet been started, simply
+        wait for an input session to start if needed.
+
+        * TestWebKitAPI/ios/UIKitSPI.h:
+
+        Add a new SPI declaration.
+
 2018-11-29  Tim Horton  <timothy_hor...@apple.com>
 
         Inform clients when editable image attachment backing data changes

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm	2018-11-30 07:22:00 UTC (rev 238728)
@@ -78,6 +78,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     [webView synchronouslyLoadHTMLString:markup];
     [webView _setEditable:YES];
+    [webView becomeFirstResponder];
     [webView stringByEvaluatingJavaScript:@"getSelection().setPosition(document.body, 1)"];
     return webView;
 }
@@ -87,6 +88,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     [webView synchronouslyLoadTestPageNamed:testPageName];
     [webView _setEditable:YES];
+    [webView becomeFirstResponder];
     [webView stringByEvaluatingJavaScript:@"getSelection().setPosition(document.body, 1)"];
     return webView;
 }
@@ -243,12 +245,11 @@
 TEST(WKWebViewEditActions, PasteAndMatchStyle)
 {
     auto source = webViewForEditActionTesting();
-    auto destination = webViewForEditActionTesting(@"<div><br></div>");
-
     [source selectAll:nil];
     [source evaluateJavaScript:@"document.execCommand('bold'); document.execCommand('underline'); document.execCommand('italic')" completionHandler:nil];
     [source _synchronouslyExecuteEditCommand:@"Copy" argument:nil];
 
+    auto destination = webViewForEditActionTesting(@"<div><br></div>");
     [destination _pasteAndMatchStyle:nil];
     [destination selectAll:nil];
     EXPECT_FALSE([destination stringByEvaluatingJavaScript:@"document.queryCommandState('bold')"].boolValue);

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html	2018-11-30 07:22:00 UTC (rev 238728)
@@ -14,6 +14,10 @@
             white-space: nowrap;
         }
 
+        #source {
+            overflow: hidden;
+        }
+
         #editor {
             border: black 1px solid;
         }

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html	2018-11-30 07:22:00 UTC (rev 238728)
@@ -14,6 +14,10 @@
             white-space: nowrap;
         }
 
+        #source {
+            overflow: hidden;
+        }
+
         #editor {
             border: black 1px solid;
         }

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2018-11-30 07:22:00 UTC (rev 238728)
@@ -98,6 +98,15 @@
 
 @end
 
+static void loadTestPageAndEnsureInputSession(DragAndDropSimulator *simulator, NSString *testPageName)
+{
+    TestWKWebView *webView = [simulator webView];
+    simulator.allowsFocusToStartInputSession = YES;
+    [webView becomeFirstResponder];
+    [webView synchronouslyLoadTestPageNamed:testPageName];
+    [simulator ensureInputSession];
+}
+
 static NSValue *makeCGRectValue(CGFloat x, CGFloat y, CGFloat width, CGFloat height)
 {
     return [NSValue valueWithCGRect:CGRectMake(x, y, width, height)];
@@ -341,8 +350,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"autofocus-contenteditable"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"autofocus-contenteditable");
     [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
 
     EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
@@ -362,8 +370,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"contenteditable-and-textarea"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"contenteditable-and-textarea");
     [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
 
     EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
@@ -394,8 +401,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"two-paragraph-contenteditable"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"two-paragraph-contenteditable");
     [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(250, 450)];
 
     NSString *finalTextContent = [webView stringByEvaluatingJavaScript:@"editor.textContent"];
@@ -425,8 +431,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"textarea-to-input"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"textarea-to-input");
     [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
 
     EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
@@ -440,8 +445,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"textarea-to-input"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"textarea-to-input");
     [webView stringByEvaluatingJavaScript:@"source.value = 'pneumonoultramicroscopicsilicovolcanoconiosis'"];
     [webView stringByEvaluatingJavaScript:@"source.selectionStart = 0"];
     [webView stringByEvaluatingJavaScript:@"source.selectionEnd = source.value.length"];
@@ -463,8 +467,7 @@
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
 
-    [webView loadTestPageNamed:@"textarea-to-input"];
-    [simulator waitForInputSession];
+    loadTestPageAndEnsureInputSession(simulator.get(), @"textarea-to-input");
     [webView stringByEvaluatingJavaScript:@"source.value = 'https://webkit.org/'"];
     [webView stringByEvaluatingJavaScript:@"source.selectionStart = 0"];
     [webView stringByEvaluatingJavaScript:@"source.selectionEnd = source.value.length"];

Modified: trunk/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h	2018-11-30 07:22:00 UTC (rev 238728)
@@ -89,7 +89,7 @@
 
 - (instancetype)initWithWebView:(TestWKWebView *)webView;
 - (void)runFrom:(CGPoint)startLocation to:(CGPoint)endLocation additionalItemRequestLocations:(ProgressToCGPointValueMap)additionalItemRequestLocations;
-- (void)waitForInputSession;
+- (void)ensureInputSession;
 
 @property (nonatomic, readonly) DragAndDropPhase phase;
 @property (nonatomic) BOOL allowsFocusToStartInputSession;

Modified: trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm	2018-11-30 07:22:00 UTC (rev 238728)
@@ -310,7 +310,7 @@
     RetainPtr<NSMutableArray<_WKAttachment *>> _insertedAttachments;
     RetainPtr<NSMutableArray<_WKAttachment *>> _removedAttachments;
 
-    bool _isDoneWaitingForInputSession;
+    bool _hasStartedInputSession;
     double _currentProgress;
     bool _isDoneWithCurrentRun;
     DragAndDropPhase _phase;
@@ -338,7 +338,6 @@
         _webView = webView;
         _shouldEnsureUIApplication = NO;
         _shouldAllowMoveOperation = YES;
-        _isDoneWaitingForInputSession = true;
         [_webView setUIDelegate:self];
         [_webView _setInputDelegate:self];
     }
@@ -373,6 +372,7 @@
     _remainingAdditionalItemRequestLocationsByProgress = nil;
     _queuedAdditionalItemRequestLocations = adoptNS([[NSMutableArray alloc] init]);
     _liftPreviews = adoptNS([[NSMutableArray alloc] init]);
+    _hasStartedInputSession = false;
 }
 
 - (NSArray *)observedEventNames
@@ -459,8 +459,13 @@
 
     [[_webView dropInteractionDelegate] dropInteraction:[_webView dropInteraction] sessionDidEnd:_dropSession.get()];
 
-    if (_dragSession)
-        [[_webView dragInteractionDelegate] dragInteraction:[_webView dragInteraction] session:_dragSession.get() didEndWithOperation:operation];
+    if (_dragSession) {
+        auto delegate = [_webView dragInteractionDelegate];
+        [delegate dragInteraction:[_webView dragInteraction] session:_dragSession.get() didEndWithOperation:operation];
+        if ([delegate respondsToSelector:@selector(_clearToken:)])
+            [(id <UITextInputMultiDocument>)delegate _clearToken:nil];
+        [_webView becomeFirstResponder];
+    }
 }
 
 - (void)_enqueuePendingAdditionalItemRequestLocations
@@ -543,8 +548,14 @@
             return;
         }
 
-        [[_webView dragInteractionDelegate] dragInteraction:[_webView dragInteraction] sessionWillBegin:_dragSession.get()];
+        auto delegate = [_webView dragInteractionDelegate];
+        if ([delegate respondsToSelector:@selector(_preserveFocusWithToken:destructively:)])
+            [(id <UITextInputMultiDocument>)delegate _preserveFocusWithToken:nil destructively:NO];
 
+        [_webView resignFirstResponder];
+
+        [delegate dragInteraction:[_webView dragInteraction] sessionWillBegin:_dragSession.get()];
+
         RetainPtr<WKWebView> retainedWebView = _webView;
         dispatch_async(dispatch_get_main_queue(), ^() {
             [retainedWebView resignFirstResponder];
@@ -618,14 +629,9 @@
     return _lastKnownDragCaretRect;
 }
 
-- (void)waitForInputSession
+- (void)ensureInputSession
 {
-    _isDoneWaitingForInputSession = false;
-
-    // Waiting for an input session implies that we should allow input sessions to begin.
-    self.allowsFocusToStartInputSession = YES;
-
-    Util::run(&_isDoneWaitingForInputSession);
+    Util::run(&_hasStartedInputSession);
 }
 
 - (NSArray<_WKAttachment *> *)insertedAttachments
@@ -707,7 +713,7 @@
 
 - (void)_webView:(WKWebView *)webView didStartInputSession:(id <_WKFormInputSession>)inputSession
 {
-    _isDoneWaitingForInputSession = true;
+    _hasStartedInputSession = true;
 }
 
 @end

Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (238727 => 238728)


--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2018-11-30 06:35:36 UTC (rev 238727)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2018-11-30 07:22:00 UTC (rev 238728)
@@ -83,6 +83,7 @@
 @optional
 - (void)_preserveFocusWithToken:(id <NSCopying, NSSecureCoding>)token destructively:(BOOL)destructively;
 - (BOOL)_restoreFocusWithToken:(id <NSCopying, NSSecureCoding>)token;
+- (void)_clearToken:(id <NSCopying, NSSecureCoding>)token;
 @end
 
 @interface NSURL ()

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to