Changeset 238188 in webkit


Ignore:
Timestamp:
Nov 14, 2018 11:27:29 AM (6 years ago)
Author:
Wenson Hsieh
Message:

[iOS] A few drag and drop tests are crashing after r238146
​https://bugs.webkit.org/show_bug.cgi?id=191617

Reviewed by Dean Jackson.

Source/WebKit:

The notion of temporarily suppressing the selection assistant was introduced during iOS drag and drop
development as a way of allowing both the selection view and dropped content snapshot to fade in simultaneously
during a drop in an editable element. r238146 piggy-backs on this mechanism by changing selection suppression
state when an element is focused, when the selection changes, and when an element is blurred, depending on
whether the currently focused element is transparent.

However, in the case where the selection assistant is suppressed due to a running drop animation, if focus moves
to an element that is not fully transparent, we end up prematurely unsuppressing the text selection assistant.
This subsequently causes selection UI to immediately show up after a drop instead of animating in alongside a
snapshot of the inserted document fragment, if the drop moved focus to an editable element.

A number of drag and drop tests on iOS exercised this codepath by dragging content into editable fields and/or
moving content between editable elements in a web view. These tests began to crash due to selection views and
the accompanying callout bar appearing earlier than usual, which triggers an unrelated UIKit assertion in
<​https://webkit.org/b/190401>.

This patch fixes the failing tests by refactoring our selection assistant suppression code. Instead of
maintaining a single BOOL flag indicating whether the selection is suppressed, we use an OptionSet of
SuppressSelectionAssistantReasons, which (at the moment) only include (1) a running drop animation, and (2)
focusing a transparent element. The text selection assistant is considered suppressed when either of the reasons
apply. This allows us to correctly handle a drop animation that occurs simultaneously as an element is focused
without unsuppressing the selection assistant early, and also allows us to handle selection assistant
suppression in more nuanced ways, depending on the suppression reason.

  • UIProcess/ios/WKContentViewInteraction.h:
  • UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView cleanupInteraction]):
(-[WKContentView _displayFormNodeInputView]):

Only prevent zooming to the focused element during drop if we're suppressing the selection assistant due to
focusing a transparent element. In the case of a drop, we still want to allow scrolling and zooming.

(-[WKContentView canShowNonEmptySelectionView]):
(-[WKContentView hasSelectablePositionAtPoint:]):
(-[WKContentView pointIsNearMarkedText:]):
(-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
(-[WKContentView _startAssistingKeyboard]):
(-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
(-[WKContentView _stopAssistingNode]):
(-[WKContentView _updateChangedSelection:]):
(-[WKContentView _shouldSuppressSelectionCommands]):
(-[WKContentView _beginSuppressingSelectionAssistantForReason:]):
(-[WKContentView _stopSuppressingSelectionAssistantForReason:]):

Add helper methods for adding or removing selection assistant suppression reasons. When the last selection
assistant suppression reason is removed, we activate the selection assistant, and conversely, when the first
suppression reason is added, we deactivate the selection assistant.

(-[WKContentView _didConcludeEditDataInteraction:]):
(-[WKContentView _didPerformDragOperation:]):
(-[WKContentView dropInteraction:performDrop:]):
(-[WKContentView suppressAssistantSelectionView]): Deleted.
(-[WKContentView setSuppressAssistantSelectionView:]): Deleted.

Tools:

Augment these crashing tests to verify that selection commands are suppressed during drop over editable elements
via more robust means. These tests currently hit an assertion when revealing the callout bar too early, because
TestWebKitAPI is not a UI application (see <​https://webkit.org/b/190401>).

Instead of relying on this other bug, directly ask the text input whether it is suppressing selection commands
during a drop, and remember the answer via DragAndDropSimulator.

  • TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:

(TestWebKitAPI::TEST):

Add to the existing tests that started failing after r238146.

  • TestWebKitAPI/cocoa/DragAndDropSimulator.h:
  • TestWebKitAPI/cocoa/TestWKWebView.h:
  • TestWebKitAPI/cocoa/TestWKWebView.mm:

(-[TestWKWebView textInputContentView]):

  • TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:

(-[DragAndDropSimulator _resetSimulatedState]):
(-[DragAndDropSimulator _webView:dataInteractionOperationWasHandled:forSession:itemProviders:]):

  • TestWebKitAPI/ios/UIKitSPI.h:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r238186 r238188  
     12018-11-14  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] A few drag and drop tests are crashing after r238146
     4        https://bugs.webkit.org/show_bug.cgi?id=191617
     5
     6        Reviewed by Dean Jackson.
     7
     8        The notion of temporarily suppressing the selection assistant was introduced during iOS drag and drop
     9        development as a way of allowing both the selection view and dropped content snapshot to fade in simultaneously
     10        during a drop in an editable element. r238146 piggy-backs on this mechanism by changing selection suppression
     11        state when an element is focused, when the selection changes, and when an element is blurred, depending on
     12        whether the currently focused element is transparent.
     13
     14        However, in the case where the selection assistant is suppressed due to a running drop animation, if focus moves
     15        to an element that is not fully transparent, we end up prematurely unsuppressing the text selection assistant.
     16        This subsequently causes selection UI to immediately show up after a drop instead of animating in alongside a
     17        snapshot of the inserted document fragment, if the drop moved focus to an editable element.
     18
     19        A number of drag and drop tests on iOS exercised this codepath by dragging content into editable fields and/or
     20        moving content between editable elements in a web view. These tests began to crash due to selection views and
     21        the accompanying callout bar appearing earlier than usual, which triggers an unrelated UIKit assertion in
     22        <https://webkit.org/b/190401>.
     23
     24        This patch fixes the failing tests by refactoring our selection assistant suppression code. Instead of
     25        maintaining a single `BOOL` flag indicating whether the selection is suppressed, we use an `OptionSet` of
     26        `SuppressSelectionAssistantReason`s, which (at the moment) only include (1) a running drop animation, and (2)
     27        focusing a transparent element. The text selection assistant is considered suppressed when either of the reasons
     28        apply. This allows us to correctly handle a drop animation that occurs simultaneously as an element is focused
     29        without unsuppressing the selection assistant early, and also allows us to handle selection assistant
     30        suppression in more nuanced ways, depending on the suppression reason.
     31
     32        * UIProcess/ios/WKContentViewInteraction.h:
     33        * UIProcess/ios/WKContentViewInteraction.mm:
     34        (-[WKContentView cleanupInteraction]):
     35        (-[WKContentView _displayFormNodeInputView]):
     36
     37        Only prevent zooming to the focused element during drop if we're suppressing the selection assistant due to
     38        focusing a transparent element. In the case of a drop, we still want to allow scrolling and zooming.
     39
     40        (-[WKContentView canShowNonEmptySelectionView]):
     41        (-[WKContentView hasSelectablePositionAtPoint:]):
     42        (-[WKContentView pointIsNearMarkedText:]):
     43        (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
     44        (-[WKContentView _startAssistingKeyboard]):
     45        (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
     46        (-[WKContentView _stopAssistingNode]):
     47        (-[WKContentView _updateChangedSelection:]):
     48        (-[WKContentView _shouldSuppressSelectionCommands]):
     49        (-[WKContentView _beginSuppressingSelectionAssistantForReason:]):
     50        (-[WKContentView _stopSuppressingSelectionAssistantForReason:]):
     51
     52        Add helper methods for adding or removing selection assistant suppression reasons. When the last selection
     53        assistant suppression reason is removed, we activate the selection assistant, and conversely, when the first
     54        suppression reason is added, we deactivate the selection assistant.
     55
     56        (-[WKContentView _didConcludeEditDataInteraction:]):
     57        (-[WKContentView _didPerformDragOperation:]):
     58        (-[WKContentView dropInteraction:performDrop:]):
     59        (-[WKContentView suppressAssistantSelectionView]): Deleted.
     60        (-[WKContentView setSuppressAssistantSelectionView:]): Deleted.
     61
    1622018-11-14  Wenson Hsieh  <wenson_hsieh@apple.com>
    263

  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

    r238186 r238188  
    5151#import <wtf/BlockPtr.h>
    5252#import <wtf/Forward.h>
     53#import <wtf/OptionSet.h>
    5354#import <wtf/Vector.h>
    5455#import <wtf/WeakObjCPtr.h>
    … …  
    157158namespace WebKit {
    158159
     160enum SuppressSelectionAssistantReason : uint8_t {
     161    FocusedElementIsTransparent = 1 << 0,
     162    DropAnimationIsRunning = 1 << 1
     163};
     164
    159165struct WKSelectionDrawingInfo {
    160166    enum class SelectionType { None, Plugin, Range };
    … …  
    210216
    211217    RetainPtr<UIWKTextInteractionAssistant> _textSelectionAssistant;
    212     BOOL _suppressAssistantSelectionView;
     218    OptionSet<WebKit::SuppressSelectionAssistantReason> _suppressSelectionAssistantReasons;
    213219
    214220    RetainPtr<UITextInputTraits> _traits;
    … …  
    338344@property (nonatomic, readonly) const WebKit::AssistedNodeInformation& assistedNodeInformation;
    339345@property (nonatomic, readonly) UIWebFormAccessory *formAccessoryView;
    340 @property (nonatomic) BOOL suppressAssistantSelectionView;
    341346@property (nonatomic, readonly) UITextInputAssistantItem *inputAssistantItemForWebView;
    342347

  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r238186 r238188  
    823823
    824824    _hasSetUpInteractions = NO;
     825    _suppressSelectionAssistantReasons = { };
    825826}
    826827
    … …  
    13431344- (void)_displayFormNodeInputView
    13441345{
    1345     if (!self.suppressAssistantSelectionView) {
     1346    if (!_suppressSelectionAssistantReasons.contains(FocusedElementIsTransparent)) {
    13461347        // In case user scaling is force enabled, do not use that scaling when zooming in with an input field.
    13471348        // Zooming above the page's default scale factor should only happen when the user performs it.
    … …  
    17371738- (BOOL)canShowNonEmptySelectionView
    17381739{
    1739     if (self.suppressAssistantSelectionView)
     1740    if (_suppressSelectionAssistantReasons)
    17401741        return NO;
    17411742
    … …  
    17491750        return NO;
    17501751
    1751     if (self.suppressAssistantSelectionView)
     1752    if (_suppressSelectionAssistantReasons)
    17521753        return NO;
    17531754
    … …  
    17751776        return NO;
    17761777
    1777     if (self.suppressAssistantSelectionView)
     1778    if (_suppressSelectionAssistantReasons)
    17781779        return NO;
    17791780
    … …  
    17891790        return NO;
    17901791
    1791     if (self.suppressAssistantSelectionView)
     1792    if (_suppressSelectionAssistantReasons)
    17921793        return NO;
    17931794
    … …  
    42004201    [self setUpTextSelectionAssistant];
    42014202   
    4202     if (self.isFirstResponder && !self.suppressAssistantSelectionView)
     4203    if (self.isFirstResponder && !_suppressSelectionAssistantReasons)
    42034204        [_textSelectionAssistant activateSelection];
    42044205
    … …  
    42864287        startInputSessionPolicy = [inputDelegate _webView:_webView decidePolicyForFocusedElement:focusedElementInfo.get()];
    42874288
    4288     self.suppressAssistantSelectionView = information.elementIsTransparent;
     4289    if (information.elementIsTransparent)
     4290        [self _beginSuppressingSelectionAssistantForReason:FocusedElementIsTransparent];
     4291    else
     4292        [self _stopSuppressingSelectionAssistantForReason:FocusedElementIsTransparent];
    42894293
    42904294    switch (startInputSessionPolicy) {
    … …  
    44314435    [_webView didEndFormControlInteraction];
    44324436
    4433     self.suppressAssistantSelectionView = NO;
     4437    [self _stopSuppressingSelectionAssistantForReason:FocusedElementIsTransparent];
    44344438}
    44354439
    … …  
    47694773
    47704774    auto& postLayoutData = state.postLayoutData();
    4771     if (hasAssistedNode(_assistedNodeInformation))
    4772         self.suppressAssistantSelectionView = postLayoutData.elementIsTransparent;
     4775    if (hasAssistedNode(_assistedNodeInformation)) {
     4776        if (postLayoutData.elementIsTransparent)
     4777            [self _beginSuppressingSelectionAssistantForReason:FocusedElementIsTransparent];
     4778        else
     4779            [self _stopSuppressingSelectionAssistantForReason:FocusedElementIsTransparent];
     4780    }
    47734781
    47744782    WKSelectionDrawingInfo selectionDrawingInfo(_page->editorState());
    … …  
    47954803        [[self selectionInteractionAssistant] showSelectionCommands];
    47964804
    4797         if (!self.suppressAssistantSelectionView)
     4805        if (!_suppressSelectionAssistantReasons)
    47984806            [_textSelectionAssistant activateSelection];
    47994807
    … …  
    48064814- (BOOL)_shouldSuppressSelectionCommands
    48074815{
    4808     return _suppressAssistantSelectionView;
    4809 }
    4810 
    4811 - (BOOL)suppressAssistantSelectionView
    4812 {
    4813     return _suppressAssistantSelectionView;
    4814 }
    4815 
    4816 - (void)setSuppressAssistantSelectionView:(BOOL)suppressAssistantSelectionView
    4817 {
    4818     if (_suppressAssistantSelectionView == suppressAssistantSelectionView)
    4819         return;
    4820 
    4821     _suppressAssistantSelectionView = suppressAssistantSelectionView;
    4822     if (!_textSelectionAssistant)
    4823         return;
    4824 
    4825     if (suppressAssistantSelectionView)
     4816    return !!_suppressSelectionAssistantReasons;
     4817}
     4818
     4819- (void)_beginSuppressingSelectionAssistantForReason:(SuppressSelectionAssistantReason)reason
     4820{
     4821    bool wasSuppressingSelectionAssistant = !!_suppressSelectionAssistantReasons;
     4822    _suppressSelectionAssistantReasons.add(reason);
     4823
     4824    if (!wasSuppressingSelectionAssistant)
    48264825        [_textSelectionAssistant deactivateSelection];
    4827     else
     4826}
     4827
     4828- (void)_stopSuppressingSelectionAssistantForReason:(SuppressSelectionAssistantReason)reason
     4829{
     4830    bool wasSuppressingSelectionAssistant = !!_suppressSelectionAssistantReasons;
     4831    _suppressSelectionAssistantReasons.remove(reason);
     4832
     4833    if (wasSuppressingSelectionAssistant && !_suppressSelectionAssistantReasons)
    48284834        [_textSelectionAssistant activateSelection];
    48294835}
    … …  
    53375343        [visibleContentViewSnapshot removeFromSuperview];
    53385344        [UIView animateWithDuration:0.25 animations:^() {
    5339             [protectedSelf setSuppressAssistantSelectionView:NO];
     5345            [protectedSelf _stopSuppressingSelectionAssistantForReason:DropAnimationIsRunning];
    53405346            [unselectedContentSnapshot setAlpha:0];
    53415347        } completion:^(BOOL completed) {
    … …  
    53545360
    53555361    if (!_isAnimatingConcludeEditDrag)
    5356         self.suppressAssistantSelectionView = NO;
     5362        [self _stopSuppressingSelectionAssistantForReason:DropAnimationIsRunning];
    53575363
    53585364    CGPoint global;
    … …  
    58035809
    58045810        retainedSelf->_visibleContentViewSnapshot = [retainedSelf snapshotViewAfterScreenUpdates:NO];
    5805         [retainedSelf setSuppressAssistantSelectionView:YES];
     5811        [retainedSelf _beginSuppressingSelectionAssistantForReason:DropAnimationIsRunning];
    58065812        [UIView performWithoutAnimation:[retainedSelf] {
    58075813            [retainedSelf->_visibleContentViewSnapshot setFrame:[retainedSelf bounds]];

  • trunk/Tools/ChangeLog

    r238186 r238188  
     12018-11-14  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] A few drag and drop tests are crashing after r238146
     4        https://bugs.webkit.org/show_bug.cgi?id=191617
     5
     6        Reviewed by Dean Jackson.
     7
     8        Augment these crashing tests to verify that selection commands are suppressed during drop over editable elements
     9        via more robust means. These tests currently hit an assertion when revealing the callout bar too early, because
     10        TestWebKitAPI is not a UI application (see <https://webkit.org/b/190401>).
     11
     12        Instead of relying on this other bug, directly ask the text input whether it is suppressing selection commands
     13        during a drop, and remember the answer via DragAndDropSimulator.
     14
     15        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
     16        (TestWebKitAPI::TEST):
     17
     18        Add to the existing tests that started failing after r238146.
     19
     20        * TestWebKitAPI/cocoa/DragAndDropSimulator.h:
     21        * TestWebKitAPI/cocoa/TestWKWebView.h:
     22        * TestWebKitAPI/cocoa/TestWKWebView.mm:
     23        (-[TestWKWebView textInputContentView]):
     24        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
     25        (-[DragAndDropSimulator _resetSimulatedState]):
     26        (-[DragAndDropSimulator _webView:dataInteractionOperationWasHandled:forSession:itemProviders:]):
     27        * TestWebKitAPI/ios/UIKitSPI.h:
     28
    1292018-11-14  Wenson Hsieh  <wenson_hsieh@apple.com>
    230

  • trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm

    r237607 r238188  
    347347    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
    348348
     349    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    349350    EXPECT_EQ([webView stringByEvaluatingJavaScript:@"source.textContent"].length, 0UL);
    350351    EXPECT_WK_STREQ("Hello world", [webView stringByEvaluatingJavaScript:@"editor.textContent"].UTF8String);
    … …  
    367368    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
    368369
     370    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    369371    EXPECT_EQ([webView stringByEvaluatingJavaScript:@"source.textContent"].length, 0UL);
    370372    EXPECT_WK_STREQ("Hello world", [webView editorValue].UTF8String);
    … …  
    402404    NSUInteger secondParagraphOffset = [finalTextContent rangeOfString:@"This is the second paragraph"].location;
    403405
     406    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    404407    EXPECT_FALSE(firstParagraphOffset == NSNotFound);
    405408    EXPECT_FALSE(secondParagraphOffset == NSNotFound);
    … …  
    428431    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
    429432
     433    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    430434    EXPECT_EQ([webView stringByEvaluatingJavaScript:@"source.value"].length, 0UL);
    431435    EXPECT_WK_STREQ("Hello world", [webView editorValue].UTF8String);
    … …  
    444448    [webView stringByEvaluatingJavaScript:@"source.selectionEnd = source.value.length"];
    445449    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
     450
     451    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    446452
    447453    NSItemProvider *itemProvider = [simulator sourceItemProviders].firstObject;
    … …  
    465471    [webView stringByEvaluatingJavaScript:@"source.selectionEnd = source.value.length"];
    466472    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
     473
     474    EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
    467475
    468476    NSItemProvider *itemProvider = [simulator sourceItemProviders].firstObject;

  • trunk/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h

    r237266 r238188  
    108108@property (nonatomic, readonly) CGRect lastKnownDragCaretRect;
    109109@property (nonatomic, readonly) NSArray<UITargetedDragPreview *> *liftPreviews;
     110@property (nonatomic, readonly) BOOL suppressedSelectionCommandsDuringDrop;
    110111
    111112#endif // PLATFORM(IOS_FAMILY)

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.h

    r238186 r238188  
    3434@class _WKActivatedElementInfo;
    3535@protocol UITextInputMultiDocument;
     36@protocol UITextInputPrivate;
    3637#endif
    3738
    … …  
    7778#if PLATFORM(IOS_FAMILY)
    7879@interface TestWKWebView (IOSOnly)
    79 @property (nonatomic, readonly) UIView <UITextInput, UITextInputMultiDocument> *textInputContentView;
     80@property (nonatomic, readonly) UIView <UITextInputPrivate, UITextInputMultiDocument> *textInputContentView;
    8081@property (nonatomic, readonly) RetainPtr<NSArray> selectionRectsAfterPresentationUpdate;
    8182@property (nonatomic, readonly) CGRect caretViewRectInContentCoordinates;

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

    r238186 r238188  
    352352@implementation TestWKWebView (IOSOnly)
    353353
    354 - (UIView <UITextInput, UITextInputMultiDocument> *)textInputContentView
    355 {
    356     return (UIView <UITextInput, UITextInputMultiDocument> *)[self valueForKey:@"_currentContentView"];
     354- (UIView <UITextInputPrivate, UITextInputMultiDocument> *)textInputContentView
     355{
     356    return (UIView <UITextInputPrivate, UITextInputMultiDocument> *)[self valueForKey:@"_currentContentView"];
    357357}
    358358

  • trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm

    r237266 r238188  
    316316    DragAndDropPhase _phase;
    317317
     318    BOOL _suppressedSelectionCommandsDuringDrop;
    318319    RetainPtr<UIDropProposal> _currentDropProposal;
    319320}
    … …  
    358359- (void)_resetSimulatedState
    359360{
     361    _suppressedSelectionCommandsDuringDrop = NO;
    360362    _phase = DragAndDropPhaseBeginning;
    361363    _currentProgress = 0;
    … …  
    651653- (void)_webView:(WKWebView *)webView dataInteractionOperationWasHandled:(BOOL)handled forSession:(id)session itemProviders:(NSArray<UIItemProvider *> *)itemProviders
    652654{
     655    _suppressedSelectionCommandsDuringDrop = [_webView textInputContentView]._shouldSuppressSelectionCommands;
    653656    _isDoneWithCurrentRun = true;
    654657

  • trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h

    r238186 r238188  
    8080- (void)insertTextSuggestion:(UITextSuggestion *)textSuggestion;
    8181- (void)handleKeyWebEvent:(WebEvent *)theEvent withCompletionHandler:(void (^)(WebEvent *, BOOL))completionHandler;
     82- (BOOL)_shouldSuppressSelectionCommands;
    8283@end
    8384
Note: See TracChangeset for help on using the changeset viewer.