REGRESSION (r238635): Dragging a text selection within WKWebView causes the selection...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 07:22:00 +0000 (07:22 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2018 07:22:00 +0000 (07:22 +0000)
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.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/PageClientImplIOS.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/autofocus-contenteditable.html
Tools/TestWebKitAPI/Tests/WebKitCocoa/contenteditable-and-textarea.html
Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm
Tools/TestWebKitAPI/ios/UIKitSPI.h

index ad765ed..8925e52 100644 (file)
@@ -1,3 +1,26 @@
+2018-11-29  Wenson Hsieh  <wenson_hsieh@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_horton@apple.com>
 
         Inform clients when editable image attachment backing data changes
index a2e39a7..968fde9 100644 (file)
@@ -166,9 +166,7 @@ bool PageClientImpl::isViewWindowActive()
 
 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()
index 73d6f2f..ead68bd 100644 (file)
@@ -1,3 +1,54 @@
+2018-11-29  Wenson Hsieh  <wenson_hsieh@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_horton@apple.com>
 
         Inform clients when editable image attachment backing data changes
index 33700e0..1019482 100644 (file)
@@ -78,6 +78,7 @@ static RetainPtr<TestWKWebView> webViewForEditActionTesting(NSString *markup)
     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 @@ static RetainPtr<TestWKWebView> webViewForEditActionTestingWithPageNamed(NSStrin
     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, PasteAsQuotation)
 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);
index e03635f..7fc2bc3 100644 (file)
             white-space: nowrap;
         }
 
+        #source {
+            overflow: hidden;
+        }
+
         #editor {
             border: black 1px solid;
         }
index 41489fc..a4b417c 100644 (file)
             white-space: nowrap;
         }
 
+        #source {
+            overflow: hidden;
+        }
+
         #editor {
             border: black 1px solid;
         }
index b879c0f..29ef674 100644 (file)
@@ -98,6 +98,15 @@ static NSData *testZIPArchive()
 
 @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 @@ TEST(DragAndDropTests, ContentEditableToContentEditable)
     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 @@ TEST(DragAndDropTests, ContentEditableToTextarea)
     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 @@ TEST(DragAndDropTests, ContentEditableMoveParagraphs)
     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 @@ TEST(DragAndDropTests, TextAreaToInput)
     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 @@ TEST(DragAndDropTests, SinglePlainTextWordTypeIdentifiers)
     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 @@ TEST(DragAndDropTests, SinglePlainTextURLTypeIdentifiers)
     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"];
index a398f98..4fe0b84 100644 (file)
@@ -89,7 +89,7 @@ typedef NSDictionary<NSNumber *, NSValue *> *ProgressToCGPointValueMap;
 
 - (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;
index 2b00c9f..3d23efc 100644 (file)
@@ -310,7 +310,7 @@ static NSArray *dragAndDropEventNames()
     RetainPtr<NSMutableArray<_WKAttachment *>> _insertedAttachments;
     RetainPtr<NSMutableArray<_WKAttachment *>> _removedAttachments;
 
-    bool _isDoneWaitingForInputSession;
+    bool _hasStartedInputSession;
     double _currentProgress;
     bool _isDoneWithCurrentRun;
     DragAndDropPhase _phase;
@@ -338,7 +338,6 @@ static NSArray *dragAndDropEventNames()
         _webView = webView;
         _shouldEnsureUIApplication = NO;
         _shouldAllowMoveOperation = YES;
-        _isDoneWaitingForInputSession = true;
         [_webView setUIDelegate:self];
         [_webView _setInputDelegate:self];
     }
@@ -373,6 +372,7 @@ static NSArray *dragAndDropEventNames()
     _remainingAdditionalItemRequestLocationsByProgress = nil;
     _queuedAdditionalItemRequestLocations = adoptNS([[NSMutableArray alloc] init]);
     _liftPreviews = adoptNS([[NSMutableArray alloc] init]);
+    _hasStartedInputSession = false;
 }
 
 - (NSArray *)observedEventNames
@@ -459,8 +459,13 @@ static NSArray *dragAndDropEventNames()
 
     [[_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,7 +548,13 @@ static NSArray *dragAndDropEventNames()
             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(), ^() {
@@ -618,14 +629,9 @@ static NSArray *dragAndDropEventNames()
     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 @@ static NSArray *dragAndDropEventNames()
 
 - (void)_webView:(WKWebView *)webView didStartInputSession:(id <_WKFormInputSession>)inputSession
 {
-    _isDoneWaitingForInputSession = true;
+    _hasStartedInputSession = true;
 }
 
 @end
index b84d945..f7bd939 100644 (file)
@@ -83,6 +83,7 @@ WTF_EXTERN_C_END
 @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 ()