Changeset 248380 in webkit

Timestamp:

Aug 7, 2019 12:05:43 PM (5 years ago)

Author:

dino@apple.com

Message:

Context menu on a universal link produces a blank preview
​https://bugs.webkit.org/show_bug.cgi?id=200485
<rdar://problem/53699620>

Reviewed by Dean Jackson.

Source/WebCore/PAL:

Define iTunesStoreURL from CoreServices.

• pal/spi/cocoa/LaunchServicesSPI.h:

Source/WebKit:

If the context menu is activated on an iTunesStore URL, pass it
on to DataDetectors, who should know how to handle it.

Two drive-by fixes:

• make it clear that early returns do not produce a value. Instead call the completion handler first, then return.
• The new API DataDetectors case doesn't need to worry about hiding link previews as DataDetectors itself will handle that.

• UIProcess/ios/WKContentViewInteraction.mm: If the URL is an iTunesStoreURL

(as defined by CoreServices), let DataDetectors handle it.
(-[WKContentView assignLegacyDataForContextMenuInteraction]):
(-[WKContentView continueContextMenuInteraction:]):
(-[WKContentView continueContextMenuInteractionWithDataDetectors:]): New method to
use DataDetectors if possible.

Location:

trunk/Source

Files:

• WebCore/PAL/ChangeLog (modified) (1 diff)
• WebCore/PAL/pal/spi/cocoa/LaunchServicesSPI.h (modified) (2 diffs)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (6 diffs)

trunk/Source/WebCore/PAL/ChangeLog

@@ +1 @@
+2019-08-06  Dean Jackson  <dino@apple.com>
+
+        Context menu on a universal link produces a blank preview
+        https://bugs.webkit.org/show_bug.cgi?id=200485
+        <rdar://problem/53699620>
+
+        Reviewed by Tim Horton.
+
+        Define iTunesStoreURL from CoreServices.
+
+        * pal/spi/cocoa/LaunchServicesSPI.h:
+
 2019-08-07  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Source/WebCore/PAL/pal/spi/cocoa/LaunchServicesSPI.h

@@ -34 +34 @@
 #elif PLATFORM(IOS_FAMILY)
 #import <MobileCoreServices/LSAppLinkPriv.h>
+#elif PLATFORM(IOS)
+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000
+#import <CoreServices/LSURLOverridePriv.h>
+#else
+#import <CoreServices/LSApplicationWorkspace.h>
+#endif
 #endif
 
-#endif
+#endif // USE(APPLE_INTERNAL_SDK)
 
 #if HAVE(APP_LINKS)
@@ -74 +80 @@
 #endif
 
+@interface NSURL ()
+- (NSURL *)iTunesStoreURL;
+@end
+
 #if PLATFORM(MAC)
 enum LSSessionID {

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-06  Dean Jackson  <dino@apple.com>
+
+        Context menu on a universal link produces a blank preview
+        https://bugs.webkit.org/show_bug.cgi?id=200485
+        <rdar://problem/53699620>
+
+        Reviewed by Tim Horton.
+
+        If the context menu is activated on an iTunesStore URL, pass it
+        on to DataDetectors, who should know how to handle it.
+
+        Two drive-by fixes:
+        - make it clear that early returns do not produce a value. Instead call the
+          completion handler first, then return.
+        - The new API DataDetectors case doesn't need to worry about hiding link previews
+          as DataDetectors itself will handle that.
+
+        * UIProcess/ios/WKContentViewInteraction.mm: If the URL is an iTunesStoreURL
+        (as defined by CoreServices), let DataDetectors handle it.
+        (-[WKContentView assignLegacyDataForContextMenuInteraction]):
+        (-[WKContentView continueContextMenuInteraction:]):
+        (-[WKContentView continueContextMenuInteractionWithDataDetectors:]): New method to
+        use DataDetectors if possible.
+
 2019-08-07  Priyanka Agarwal  <pagarwal999@apple.com>
 

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

@@ -107 +107 @@
 #import <pal/spi/cg/CoreGraphicsSPI.h>
 #import <pal/spi/cocoa/DataDetectorsCoreSPI.h>
+#import <pal/spi/cocoa/LaunchServicesSPI.h>
 #import <pal/spi/ios/DataDetectorsUISPI.h>
 #import <pal/spi/ios/GraphicsServicesSPI.h>
@@ -7809 +7810 @@
         // Previously, UIPreviewItemController would detect the case where there was no previewViewController
         // and create one. We need to replicate this code for the new API.
-        if (!previewViewController) {
+        if (!previewViewController || [(NSURL *)url iTunesStoreURL]) {
             auto ddContextMenuActionClass = getDDContextMenuActionClass();
             if ([ddContextMenuActionClass respondsToSelector:@selector(contextMenuConfigurationForURL:identifier:selectedText:results:inView:context:menuIdentifier:)]) {
@@ -7954 +7955 @@
         _page->startInteractionWithElementAtPosition(_positionInformation.request.point);
 
-        // FIXME: Should we provide an identifier and ASSERT in delegates if we don't match?
-        return continueWithContextMenuConfiguration([UIContextMenuConfiguration configurationWithIdentifier:nil previewProvider:contentPreviewProvider actionProvider:actionMenuProvider]);
-    }
+        continueWithContextMenuConfiguration([UIContextMenuConfiguration configurationWithIdentifier:nil previewProvider:contentPreviewProvider actionProvider:actionMenuProvider]);
+        return;
+    }
+
+#if ENABLE(DATA_DETECTION)
+    if ([(NSURL *)linkURL iTunesStoreURL]) {
+        if ([self continueContextMenuInteractionWithDataDetectors:continueWithContextMenuConfiguration])
+            return;
+    }
+#endif
 
     auto completionBlock = makeBlockPtr([continueWithContextMenuConfiguration = makeBlockPtr(continueWithContextMenuConfiguration), linkURL = WTFMove(linkURL), weakSelf = WeakObjCPtr<WKContentView>(self)] (UIContextMenuConfiguration *configurationFromWKUIDelegate) mutable {
 
         auto strongSelf = weakSelf.get();
-        if (!strongSelf)
-            return continueWithContextMenuConfiguration(nil);
+        if (!strongSelf) {
+            continueWithContextMenuConfiguration(nil);
+            return;
+        }
 
         if (configurationFromWKUIDelegate) {
@@ -7997 +8007 @@
             };
 
-            return continueWithContextMenuConfiguration([UIContextMenuConfiguration configurationWithIdentifier:nil previewProvider:contentPreviewProvider actionProvider:actionMenuProvider]);
+            continueWithContextMenuConfiguration([UIContextMenuConfiguration configurationWithIdentifier:nil previewProvider:contentPreviewProvider actionProvider:actionMenuProvider]);
+            return;
         }
 
@@ -8008 +8019 @@
         // FIXME: Support JavaScript urls here. But make sure they don't show a preview.
         // <rdar://problem/50572283>
-        if (!linkURL.protocolIsInHTTPFamily() && !WebCore::DataDetection::canBePresentedByDataDetectors(linkURL))
-            return continueWithContextMenuConfiguration(nil);
-
-        BEGIN_BLOCK_OBJC_EXCEPTIONS;
-        auto ddContextMenuActionClass = getDDContextMenuActionClass();
-        if ([ddContextMenuActionClass respondsToSelector:@selector(contextMenuConfigurationWithURL:inView:context:menuIdentifier:)]) {
-            NSDictionary *context = [strongSelf dataDetectionContextForPositionInformation:strongSelf->_positionInformation];
-            UIContextMenuConfiguration *configurationFromDD = [ddContextMenuActionClass contextMenuConfigurationForURL:linkURL identifier:strongSelf->_positionInformation.dataDetectorIdentifier selectedText:[strongSelf selectedText] results:strongSelf->_positionInformation.dataDetectorResults.get() inView:strongSelf.get() context:context menuIdentifier:nil];
-            strongSelf->_contextMenuActionProviderDelegateNeedsOverride = YES;
-            strongSelf->_page->startInteractionWithElementAtPosition(strongSelf->_positionInformation.request.point);
-            if (strongSelf->_showLinkPreviews)
-                return continueWithContextMenuConfiguration(configurationFromDD);
-            return continueWithContextMenuConfiguration([UIContextMenuConfiguration configurationWithIdentifier:[configurationFromDD identifier] previewProvider:nil actionProvider:[configurationFromDD actionProvider]]);
+        if (!linkURL.protocolIsInHTTPFamily() && !WebCore::DataDetection::canBePresentedByDataDetectors(linkURL)) {
+            continueWithContextMenuConfiguration(nil);
+            return;
         }
-        END_BLOCK_OBJC_EXCEPTIONS;
-#endif
-        return continueWithContextMenuConfiguration(nil);
+
+        if ([strongSelf continueContextMenuInteractionWithDataDetectors:continueWithContextMenuConfiguration.get()])
+            return;
+#endif
+        continueWithContextMenuConfiguration(nil);
     });
 
@@ -8048 +8051 @@
         completionBlock(nil);
 }
+
+#if ENABLE(DATA_DETECTION)
+- (BOOL)continueContextMenuInteractionWithDataDetectors:(void(^)(UIContextMenuConfiguration *))continueWithContextMenuConfiguration
+{
+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
+    auto ddContextMenuActionClass = getDDContextMenuActionClass();
+    if ([ddContextMenuActionClass respondsToSelector:@selector(contextMenuConfigurationWithURL:inView:context:menuIdentifier:)]) {
+        URL linkURL = _positionInformation.url;
+        NSDictionary *context = [self dataDetectionContextForPositionInformation:_positionInformation];
+        UIContextMenuConfiguration *configurationFromDD = [ddContextMenuActionClass contextMenuConfigurationForURL:linkURL identifier:_positionInformation.dataDetectorIdentifier selectedText:[self selectedText] results:_positionInformation.dataDetectorResults.get() inView:self context:context menuIdentifier:nil];
+        _contextMenuActionProviderDelegateNeedsOverride = YES;
+        _page->startInteractionWithElementAtPosition(_positionInformation.request.point);
+        continueWithContextMenuConfiguration(configurationFromDD);
+        return YES;
+    }
+    END_BLOCK_OBJC_EXCEPTIONS;
+
+    return NO;
+}
+#endif
 
 - (NSArray<UIMenuElement *> *)_contextMenuInteraction:(UIContextMenuInteraction *)interaction overrideSuggestedActionsForConfiguration:(UIContextMenuConfiguration *)configuration