Title: [258659] trunk
- Revision
- 258659
- Author
- [email protected]
- Date
- 2020-03-18 12:31:20 -0700 (Wed, 18 Mar 2020)
Log 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]):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (258658 => 258659)
--- trunk/Source/WebKit/ChangeLog 2020-03-18 19:28:39 UTC (rev 258658)
+++ trunk/Source/WebKit/ChangeLog 2020-03-18 19:31:20 UTC (rev 258659)
@@ -1,3 +1,50 @@
+2020-03-18 Wenson Hsieh <[email protected]>
+
+ 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.
+
+ 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).
+
2020-03-18 Chris Dumez <[email protected]>
Unreviewed, fix iOS build with recent SDKs.
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (258658 => 258659)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2020-03-18 19:28:39 UTC (rev 258658)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2020-03-18 19:31:20 UTC (rev 258659)
@@ -246,6 +246,7 @@
RetainPtr<UIView> _interactionViewsContainerView;
RetainPtr<UIView> _contextMenuHintContainerView;
RetainPtr<UIView> _dragPreviewContainerView;
+ RetainPtr<UIView> _dropPreviewContainerView;
RetainPtr<NSString> _markedText;
RetainPtr<WKActionSheetAssistant> _actionSheetAssistant;
#if ENABLE(AIRPLAY_PICKER)
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (258658 => 258659)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-03-18 19:28:39 UTC (rev 258658)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-03-18 19:31:20 UTC (rev 258659)
@@ -6814,40 +6814,48 @@
return WebCore::IOSApplication::isDataActivation();
}
+- (RetainPtr<UIView>)_createPreviewContainerWithLayerName:(NSString *)layerName
+{
+ auto container = adoptNS([[UIView alloc] init]);
+ [container layer].anchorPoint = CGPointZero;
+ [container layer].name = layerName;
+ [_interactionViewsContainerView addSubview:container.get()];
+ return container;
+}
+
+- (UIView *)containerForDropPreviews
+{
+ if (!_dropPreviewContainerView)
+ _dropPreviewContainerView = [self _createPreviewContainerWithLayerName:@"Drop Preview Container"];
+
+ ASSERT([_dropPreviewContainerView superview]);
+ [_dropPreviewContainerView setHidden:NO];
+ return _dropPreviewContainerView.get();
+}
+
- (UIView *)containerForDragPreviews
{
- if (_dragPreviewContainerView) {
- ASSERT([_dragPreviewContainerView superview]);
- [_dragPreviewContainerView setHidden:NO];
- return _dragPreviewContainerView.get();
- }
+ if (!_dragPreviewContainerView)
+ _dragPreviewContainerView = [self _createPreviewContainerWithLayerName:@"Drag Preview Container"];
- _dragPreviewContainerView = adoptNS([[UIView alloc] init]);
- [_dragPreviewContainerView layer].anchorPoint = CGPointZero;
- [_dragPreviewContainerView layer].name = @"Drag Preview Container";
- [_interactionViewsContainerView addSubview:_dragPreviewContainerView.get()];
-
+ ASSERT([_dragPreviewContainerView superview]);
+ [_dragPreviewContainerView setHidden:NO];
return _dragPreviewContainerView.get();
}
- (UIView *)containerForContextMenuHintPreviews
{
- if (_contextMenuHintContainerView) {
- ASSERT([_contextMenuHintContainerView superview]);
- [_contextMenuHintContainerView setHidden:NO];
- return _contextMenuHintContainerView.get();
- }
+ if (!_contextMenuHintContainerView)
+ _contextMenuHintContainerView = [self _createPreviewContainerWithLayerName:@"Context Menu Hint Preview Container"];
- _contextMenuHintContainerView = adoptNS([[UIView alloc] init]);
- [_contextMenuHintContainerView layer].anchorPoint = CGPointZero;
- [_contextMenuHintContainerView layer].name = @"Context Menu Hint Container";
- [_interactionViewsContainerView addSubview:_contextMenuHintContainerView.get()];
-
+ ASSERT([_contextMenuHintContainerView superview]);
+ [_contextMenuHintContainerView setHidden:NO];
return _contextMenuHintContainerView.get();
}
- (void)_hideTargetedPreviewContainerViews
{
+ [_dropPreviewContainerView setHidden:YES];
[_dragPreviewContainerView setHidden:YES];
[_contextMenuHintContainerView setHidden:YES];
}
@@ -7144,7 +7152,7 @@
[_unselectedContentSnapshot setFrame:data->contentImageWithoutSelectionRectInRootViewCoordinates];
[self insertSubview:_unselectedContentSnapshot.get() belowSubview:_visibleContentViewSnapshot.get()];
- _dragDropInteractionState.deliverDelayedDropPreview(self, self.containerForDragPreviews, data.value());
+ _dragDropInteractionState.deliverDelayedDropPreview(self, self.containerForDropPreviews, data.value());
}
- (void)_didPerformDragOperation:(BOOL)handled
@@ -7833,6 +7841,7 @@
- (void)dropInteraction:(UIDropInteraction *)interaction concludeDrop:(id <UIDropSession>)session
{
+ [std::exchange(_dropPreviewContainerView, nil) removeFromSuperview];
[std::exchange(_visibleContentViewSnapshot, nil) removeFromSuperview];
[std::exchange(_unselectedContentSnapshot, nil) removeFromSuperview];
_dragDropInteractionState.clearAllDelayedItemPreviewProviders();
Modified: trunk/Tools/ChangeLog (258658 => 258659)
--- trunk/Tools/ChangeLog 2020-03-18 19:28:39 UTC (rev 258658)
+++ trunk/Tools/ChangeLog 2020-03-18 19:31:20 UTC (rev 258659)
@@ -1,3 +1,19 @@
+2020-03-18 Wenson Hsieh <[email protected]>
+
+ 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.
+
+ 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]):
+
2020-03-18 Aakash Jain <[email protected]>
[ews] commit-queue should comment on bug in case of test failures
Modified: trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm (258658 => 258659)
--- trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm 2020-03-18 19:28:39 UTC (rev 258658)
+++ trunk/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm 2020-03-18 19:31:20 UTC (rev 258659)
@@ -23,13 +23,14 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include "config.h"
-#include "DragAndDropSimulator.h"
+#import "config.h"
+#import "DragAndDropSimulator.h"
#if ENABLE(DRAG_SUPPORT) && PLATFORM(IOS_FAMILY) && !PLATFORM(MACCATALYST)
#import "InstanceMethodSwizzler.h"
#import "PlatformUtilities.h"
+#import "Test.h"
#import "UIKitSPI.h"
#import <UIKit/UIDragInteraction.h>
#import <UIKit/UIDragItem.h>
@@ -500,11 +501,16 @@
if (preview)
[_delayedDropPreviews setObject:preview atIndexedSubscript:dropPreviewIndex];
- if (!--numberOfPendingPreviews)
+ if (!--numberOfPendingPreviews) {
_isDoneWaitingForDelayedDropPreviews = true;
+ [self _expectNoDropPreviewsWithUnparentedContainerViews];
+ }
}];
++dropPreviewIndex;
}
+
+ [self _expectNoDropPreviewsWithUnparentedContainerViews];
+
[[_webView dropInteractionDelegate] dropInteraction:[_webView dropInteraction] performDrop:_dropSession.get()];
_phase = DragAndDropPhasePerformingDrop;
@@ -826,8 +832,32 @@
_dropAnimationCompletionBlocks.append(makeBlockPtr(completion));
}
+- (void)_expectNoDropPreviewsWithUnparentedContainerViews
+{
+ auto checkDropPreview = [&](id dropPreviewOrNull) {
+ if (![dropPreviewOrNull isKindOfClass:UITargetedPreview.class])
+ return;
+
+ auto *previewContainer = [(UITargetedDragPreview *)dropPreviewOrNull target].container;
+ if (!previewContainer)
+ return;
+
+ if ([previewContainer isKindOfClass:UIWindow.class])
+ return;
+
+ EXPECT_NOT_NULL(previewContainer.window);
+ };
+
+ for (id dropPreviewOrNull in _dropPreviews.get())
+ checkDropPreview(dropPreviewOrNull);
+
+ for (id dropPreviewOrNull in _delayedDropPreviews.get())
+ checkDropPreview(dropPreviewOrNull);
+}
+
- (void)_invokeDropAnimationCompletionBlocksAndConcludeDrop
{
+ [self _expectNoDropPreviewsWithUnparentedContainerViews];
for (auto block : std::exchange(_dropAnimationCompletionBlocks, { }))
block(UIViewAnimatingPositionEnd);
[[_webView dropInteractionDelegate] dropInteraction:[_webView dropInteraction] concludeDrop:_dropSession.get()];
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
