Changeset 258659 in webkit


Ignore:
Timestamp:
Mar 18, 2020 12:31:20 PM (4 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (r257214): Targeted preview animates to the wrong place when dropping in editable content
​https://bugs.webkit.org/show_bug.cgi?id=209218
<rdar://problem/60560831>

Reviewed by Tim Horton.

Source/WebKit:

In r257214, we split out the context menu hint preview container view into two views: one for drag and drop, and
another for the context menu hint. The container view used for both drag and drop previews was removed under
-cleanUpDragSourceSessionState, which is invoked after both drag and drop sessions have ended; however, in the
case of a drop in editable content where the drop preview is delayed, the drop animation can end up finishing
after -cleanUpDragSourceSessionState is invoked. This means we end up prematurely unparenting the preview
container, which results in a broken drop animation.

To fix this, split the drag and drop container views further, into separate container views for dragging and for
dropping. The drag preview container will continue to be removed under -cleanUpDragSourceSessionState, and the
drop preview container will now be removed under the delegate call to -dropInteraction:concludeDrop:, which is
invoked by UIKit after all drop previews are finished animating.

Covered by adding additional test assertions while running existing API tests (see Tools/ChangeLog for more
details).

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

(-[WKContentView _createPreviewContainerWithLayerName:]):

Pull out common logic for creating and setting up a preview container view into a helper method. This is used by
the three methods below, which ensure container views for each of the types of previews we create when showing
the context menu, dragging an element, and dropping.

(-[WKContentView containerForDropPreviews]):
(-[WKContentView containerForDragPreviews]):
(-[WKContentView containerForContextMenuHintPreviews]):

Add a third preview container view for drop previews, and factor duplicated code in these three methods into a
common helper (see above).

(-[WKContentView _hideTargetedPreviewContainerViews]):
(-[WKContentView _deliverDelayedDropPreviewIfPossible:]):

Instead of using the container for drag previews, use the container for drop previews.

(-[WKContentView dropInteraction:concludeDrop:]):

Remove the drop preview container after the drop has concluded (i.e. all animations are complete).

Tools:

Augment the drag and drop test harness to verify that for all targeted previews, if they have container views,
those views must be parented (i.e. they must be connected to a UIWindow).

  • TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:

(-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
(-[DragAndDropSimulator _expectNoDropPreviewsWithUnparentedContainerViews]):
(-[DragAndDropSimulator _invokeDropAnimationCompletionBlocksAndConcludeDrop]):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r258654 r258659  
     12020-03-18  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r257214): Targeted preview animates to the wrong place when dropping in editable content
     4        https://bugs.webkit.org/show_bug.cgi?id=209218
     5        <rdar://problem/60560831>
     6
     7        Reviewed by Tim Horton.
     8
     9        In r257214, we split out the context menu hint preview container view into two views: one for drag and drop, and
     10        another for the context menu hint. The container view used for both drag and drop previews was removed under
     11        -cleanUpDragSourceSessionState, which is invoked after both drag and drop sessions have ended; however, in the
     12        case of a drop in editable content where the drop preview is delayed, the drop animation can end up finishing
     13        after -cleanUpDragSourceSessionState is invoked. This means we end up prematurely unparenting the preview
     14        container, which results in a broken drop animation.
     15
     16        To fix this, split the drag and drop container views further, into separate container views for dragging and for
     17        dropping. The drag preview container will continue to be removed under -cleanUpDragSourceSessionState, and the
     18        drop preview container will now be removed under the delegate call to -dropInteraction:concludeDrop:, which is
     19        invoked by UIKit after all drop previews are finished animating.
     20
     21        Covered by adding additional test assertions while running existing API tests (see Tools/ChangeLog for more
     22        details).
     23
     24        * UIProcess/ios/WKContentViewInteraction.h:
     25        * UIProcess/ios/WKContentViewInteraction.mm:
     26        (-[WKContentView _createPreviewContainerWithLayerName:]):
     27
     28        Pull out common logic for creating and setting up a preview container view into a helper method. This is used by
     29        the three methods below, which ensure container views for each of the types of previews we create when showing
     30        the context menu, dragging an element, and dropping.
     31
     32        (-[WKContentView containerForDropPreviews]):
     33        (-[WKContentView containerForDragPreviews]):
     34        (-[WKContentView containerForContextMenuHintPreviews]):
     35
     36        Add a third preview container view for drop previews, and factor duplicated code in these three methods into a
     37        common helper (see above).
     38
     39        (-[WKContentView _hideTargetedPreviewContainerViews]):
     40        (-[WKContentView _deliverDelayedDropPreviewIfPossible:]):
     41
     42        Instead of using the container for drag previews, use the container for drop previews.
     43
     44        (-[WKContentView dropInteraction:concludeDrop:]):
     45
     46        Remove the drop preview container after the drop has concluded (i.e. all animations are complete).
     47
    1482020-03-18  Chris Dumez  <cdumez@apple.com>
    249

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

    r258148 r258659  
    247247    RetainPtr<UIView> _contextMenuHintContainerView;
    248248    RetainPtr<UIView> _dragPreviewContainerView;
     249    RetainPtr<UIView> _dropPreviewContainerView;
    249250    RetainPtr<NSString> _markedText;
    250251    RetainPtr<WKActionSheetAssistant> _actionSheetAssistant;

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

    r258653 r258659  
    68156815}
    68166816
     6817- (RetainPtr<UIView>)_createPreviewContainerWithLayerName:(NSString *)layerName
     6818{
     6819    auto container = adoptNS([[UIView alloc] init]);
     6820    [container layer].anchorPoint = CGPointZero;
     6821    [container layer].name = layerName;
     6822    [_interactionViewsContainerView addSubview:container.get()];
     6823    return container;
     6824}
     6825
     6826- (UIView *)containerForDropPreviews
     6827{
     6828    if (!_dropPreviewContainerView)
     6829        _dropPreviewContainerView = [self _createPreviewContainerWithLayerName:@"Drop Preview Container"];
     6830
     6831    ASSERT([_dropPreviewContainerView superview]);
     6832    [_dropPreviewContainerView setHidden:NO];
     6833    return _dropPreviewContainerView.get();
     6834}
     6835
    68176836- (UIView *)containerForDragPreviews
    68186837{
    6819     if (_dragPreviewContainerView) {
    6820         ASSERT([_dragPreviewContainerView superview]);
    6821         [_dragPreviewContainerView setHidden:NO];
    6822         return _dragPreviewContainerView.get();
    6823     }
    6824 
    6825     _dragPreviewContainerView = adoptNS([[UIView alloc] init]);
    6826     [_dragPreviewContainerView layer].anchorPoint = CGPointZero;
    6827     [_dragPreviewContainerView layer].name = @"Drag Preview Container";
    6828     [_interactionViewsContainerView addSubview:_dragPreviewContainerView.get()];
    6829 
     6838    if (!_dragPreviewContainerView)
     6839        _dragPreviewContainerView = [self _createPreviewContainerWithLayerName:@"Drag Preview Container"];
     6840
     6841    ASSERT([_dragPreviewContainerView superview]);
     6842    [_dragPreviewContainerView setHidden:NO];
    68306843    return _dragPreviewContainerView.get();
    68316844}
    … …  
    68336846- (UIView *)containerForContextMenuHintPreviews
    68346847{
    6835     if (_contextMenuHintContainerView) {
    6836         ASSERT([_contextMenuHintContainerView superview]);
    6837         [_contextMenuHintContainerView setHidden:NO];
    6838         return _contextMenuHintContainerView.get();
    6839     }
    6840 
    6841     _contextMenuHintContainerView = adoptNS([[UIView alloc] init]);
    6842     [_contextMenuHintContainerView layer].anchorPoint = CGPointZero;
    6843     [_contextMenuHintContainerView layer].name = @"Context Menu Hint Container";
    6844     [_interactionViewsContainerView addSubview:_contextMenuHintContainerView.get()];
    6845 
     6848    if (!_contextMenuHintContainerView)
     6849        _contextMenuHintContainerView = [self _createPreviewContainerWithLayerName:@"Context Menu Hint Preview Container"];
     6850
     6851    ASSERT([_contextMenuHintContainerView superview]);
     6852    [_contextMenuHintContainerView setHidden:NO];
    68466853    return _contextMenuHintContainerView.get();
    68476854}
    … …  
    68496856- (void)_hideTargetedPreviewContainerViews
    68506857{
     6858    [_dropPreviewContainerView setHidden:YES];
    68516859    [_dragPreviewContainerView setHidden:YES];
    68526860    [_contextMenuHintContainerView setHidden:YES];
    … …  
    71457153
    71467154    [self insertSubview:_unselectedContentSnapshot.get() belowSubview:_visibleContentViewSnapshot.get()];
    7147     _dragDropInteractionState.deliverDelayedDropPreview(self, self.containerForDragPreviews, data.value());
     7155    _dragDropInteractionState.deliverDelayedDropPreview(self, self.containerForDropPreviews, data.value());
    71487156}
    71497157
    … …  
    78347842- (void)dropInteraction:(UIDropInteraction *)interaction concludeDrop:(id <UIDropSession>)session
    78357843{
     7844    [std::exchange(_dropPreviewContainerView, nil) removeFromSuperview];
    78367845    [std::exchange(_visibleContentViewSnapshot, nil) removeFromSuperview];
    78377846    [std::exchange(_unselectedContentSnapshot, nil) removeFromSuperview];

  • trunk/Tools/ChangeLog

    r258652 r258659  
     12020-03-18  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r257214): Targeted preview animates to the wrong place when dropping in editable content
     4        https://bugs.webkit.org/show_bug.cgi?id=209218
     5        <rdar://problem/60560831>
     6
     7        Reviewed by Tim Horton.
     8
     9        Augment the drag and drop test harness to verify that for all targeted previews, if they have container views,
     10        those views must be parented (i.e. they must be connected to a UIWindow).
     11
     12        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
     13        (-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
     14        (-[DragAndDropSimulator _expectNoDropPreviewsWithUnparentedContainerViews]):
     15        (-[DragAndDropSimulator _invokeDropAnimationCompletionBlocksAndConcludeDrop]):
     16
    1172020-03-18  Aakash Jain  <aakash_jain@apple.com>
    218

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

    r255907 r258659  
    2424 */
    2525
    26 #include "config.h"
    27 #include "DragAndDropSimulator.h"
     26#import "config.h"
     27#import "DragAndDropSimulator.h"
    2828
    2929#if ENABLE(DRAG_SUPPORT) && PLATFORM(IOS_FAMILY) && !PLATFORM(MACCATALYST)
    … …  
    3131#import "InstanceMethodSwizzler.h"
    3232#import "PlatformUtilities.h"
     33#import "Test.h"
    3334#import "UIKitSPI.h"
    3435#import <UIKit/UIDragInteraction.h>
    … …  
    501502                    [_delayedDropPreviews setObject:preview atIndexedSubscript:dropPreviewIndex];
    502503
    503                 if (!--numberOfPendingPreviews)
     504                if (!--numberOfPendingPreviews) {
    504505                    _isDoneWaitingForDelayedDropPreviews = true;
     506                    [self _expectNoDropPreviewsWithUnparentedContainerViews];
     507                }
    505508            }];
    506509            ++dropPreviewIndex;
    507510        }
     511
     512        [self _expectNoDropPreviewsWithUnparentedContainerViews];
     513
    508514        [[_webView dropInteractionDelegate] dropInteraction:[_webView dropInteraction] performDrop:_dropSession.get()];
    509515        _phase = DragAndDropPhasePerformingDrop;
    … …  
    827833}
    828834
     835- (void)_expectNoDropPreviewsWithUnparentedContainerViews
     836{
     837    auto checkDropPreview = [&](id dropPreviewOrNull) {
     838        if (![dropPreviewOrNull isKindOfClass:UITargetedPreview.class])
     839            return;
     840
     841        auto *previewContainer = [(UITargetedDragPreview *)dropPreviewOrNull target].container;
     842        if (!previewContainer)
     843            return;
     844
     845        if ([previewContainer isKindOfClass:UIWindow.class])
     846            return;
     847
     848        EXPECT_NOT_NULL(previewContainer.window);
     849    };
     850
     851    for (id dropPreviewOrNull in _dropPreviews.get())
     852        checkDropPreview(dropPreviewOrNull);
     853
     854    for (id dropPreviewOrNull in _delayedDropPreviews.get())
     855        checkDropPreview(dropPreviewOrNull);
     856}
     857
    829858- (void)_invokeDropAnimationCompletionBlocksAndConcludeDrop
    830859{
     860    [self _expectNoDropPreviewsWithUnparentedContainerViews];
    831861    for (auto block : std::exchange(_dropAnimationCompletionBlocks, { }))
    832862        block(UIViewAnimatingPositionEnd);
Note: See TracChangeset for help on using the changeset viewer.