- Revision
- 278459
- Author
- [email protected]
- Date
- 2021-06-04 08:32:21 -0700 (Fri, 04 Jun 2021)
Log Message
[iOS] Long pressing images on 9gag.com fails to present context menus
https://bugs.webkit.org/show_bug.cgi?id=226625
rdar://78136095
Reviewed by Megan Gardner.
Source/WebKit:
Make some small adjustments to context menu logic to allow the context menu to appear when long pressing images
that are inside links (anchor elements) with _javascript_ URLs. See below for more details.
Test: fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html
* UIProcess/ios/WKActionSheetAssistant.mm:
(isJavaScriptURL):
Pull logic that checks for "_javascript_" URL schemes into a separate helper function, and use it below.
(-[WKActionSheetAssistant _createSheetWithElementActions:defaultTitle:showLinkTitle:]):
(-[WKActionSheetAssistant defaultActionsForLinkSheet:]):
(-[WKActionSheetAssistant defaultActionsForImageSheet:]):
Add `_WKElementActionTypeCopy` for image sheets, even when the target URL is a _javascript_ URL; even if the image
is inside a _javascript_ URL, we should be capable of triggering the "Copy" action by copying the image data and
the image URL, rather than the _javascript_ URL. The change in `WebPage::performActionOnElement` below ensures
that we don't end up copying the _javascript_ URL.
(-[WKActionSheetAssistant handleElementActionWithType:element:needsInteraction:]):
Make another minor adjustment to share the imageURL instead of the target URL in the case where the target is a
_javascript_ URL.
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView continueContextMenuInteraction:]):
Avoid bailing early with `continueWithContextMenuConfiguration(nil);` in the case where we're long pressing an
image inside a _javascript_ link.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::performActionOnElement):
Push the _javascript_ URL check in `defaultActionsForImageSheet` down into `WebPage::performActionOnElement`, when
copying an image. This tweak allows us to still show the Copy action for images in _javascript_ URLs, but just
copy the image and image URL instead of the _javascript_ URL.
LayoutTests:
Add a layout test to verify that a context menu is presented when long pressing an image inside a link with a
_javascript_ URL as an `href`.
* fast/events/touch/ios/long-press-on-image-in-_javascript_-link-expected.txt: Added.
* fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (278458 => 278459)
--- trunk/LayoutTests/ChangeLog 2021-06-04 15:03:00 UTC (rev 278458)
+++ trunk/LayoutTests/ChangeLog 2021-06-04 15:32:21 UTC (rev 278459)
@@ -1,3 +1,17 @@
+2021-06-04 Wenson Hsieh <[email protected]>
+
+ [iOS] Long pressing images on 9gag.com fails to present context menus
+ https://bugs.webkit.org/show_bug.cgi?id=226625
+ rdar://78136095
+
+ Reviewed by Megan Gardner.
+
+ Add a layout test to verify that a context menu is presented when long pressing an image inside a link with a
+ _javascript_ URL as an `href`.
+
+ * fast/events/touch/ios/long-press-on-image-in-_javascript_-link-expected.txt: Added.
+ * fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html: Added.
+
2021-06-04 Martin Robinson <[email protected]>
[Win] Implement scroll-snap-points on Windows
Added: trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link-expected.txt (0 => 278459)
--- trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link-expected.txt 2021-06-04 15:32:21 UTC (rev 278459)
@@ -0,0 +1,11 @@
+Verifies that long pressing an image inside a _javascript_ link presents a context menu. To manually test, long press on the image.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Presented context menu
+PASS Dismissed context menu
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html (0 => 278459)
--- trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html 2021-06-04 15:32:21 UTC (rev 278459)
@@ -0,0 +1,44 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<meta charset="utf-8">
+<html>
+<head>
+<meta name="viewport" content="initial-scale=1, width=device-width, user-scalable=no">
+<script src=""
+<script src=""
+<script src=""
+<style>
+#target {
+ width: 320px;
+ height: 240px;
+ background-color: silver;
+ display: block;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+ description("Verifies that long pressing an image inside a _javascript_ link presents a context menu. To manually test, long press on the image.");
+
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ const rect = document.getElementById("target").getBoundingClientRect();
+ await UIHelper.longPressAtPoint(rect.x + rect.width / 2, rect.y + rect.height / 2);
+ await UIHelper.waitForContextMenuToShow();
+ testPassed("Presented context menu");
+
+ await UIHelper.activateAt(0, 0);
+ await UIHelper.waitForContextMenuToHide();
+ testPassed("Dismissed context menu");
+
+ finishJSTest();
+});
+</script>
+</head>
+<body>
+<a id="target" href=""
+ <img src="" width="320" height="240">
+</a>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebKit/ChangeLog (278458 => 278459)
--- trunk/Source/WebKit/ChangeLog 2021-06-04 15:03:00 UTC (rev 278458)
+++ trunk/Source/WebKit/ChangeLog 2021-06-04 15:32:21 UTC (rev 278459)
@@ -1,3 +1,48 @@
+2021-06-04 Wenson Hsieh <[email protected]>
+
+ [iOS] Long pressing images on 9gag.com fails to present context menus
+ https://bugs.webkit.org/show_bug.cgi?id=226625
+ rdar://78136095
+
+ Reviewed by Megan Gardner.
+
+ Make some small adjustments to context menu logic to allow the context menu to appear when long pressing images
+ that are inside links (anchor elements) with _javascript_ URLs. See below for more details.
+
+ Test: fast/events/touch/ios/long-press-on-image-in-_javascript_-link.html
+
+ * UIProcess/ios/WKActionSheetAssistant.mm:
+ (isJavaScriptURL):
+
+ Pull logic that checks for "_javascript_" URL schemes into a separate helper function, and use it below.
+
+ (-[WKActionSheetAssistant _createSheetWithElementActions:defaultTitle:showLinkTitle:]):
+ (-[WKActionSheetAssistant defaultActionsForLinkSheet:]):
+ (-[WKActionSheetAssistant defaultActionsForImageSheet:]):
+
+ Add `_WKElementActionTypeCopy` for image sheets, even when the target URL is a _javascript_ URL; even if the image
+ is inside a _javascript_ URL, we should be capable of triggering the "Copy" action by copying the image data and
+ the image URL, rather than the _javascript_ URL. The change in `WebPage::performActionOnElement` below ensures
+ that we don't end up copying the _javascript_ URL.
+
+ (-[WKActionSheetAssistant handleElementActionWithType:element:needsInteraction:]):
+
+ Make another minor adjustment to share the imageURL instead of the target URL in the case where the target is a
+ _javascript_ URL.
+
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView continueContextMenuInteraction:]):
+
+ Avoid bailing early with `continueWithContextMenuConfiguration(nil);` in the case where we're long pressing an
+ image inside a _javascript_ link.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::performActionOnElement):
+
+ Push the _javascript_ URL check in `defaultActionsForImageSheet` down into `WebPage::performActionOnElement`, when
+ copying an image. This tweak allows us to still show the Copy action for images in _javascript_ URLs, but just
+ copy the image and image URL instead of the _javascript_ URL.
+
2021-06-04 Michael Catanzaro <[email protected]>
Fix more GCC warnings
Modified: trunk/Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm (278458 => 278459)
--- trunk/Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm 2021-06-04 15:03:00 UTC (rev 278458)
+++ trunk/Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm 2021-06-04 15:32:21 UTC (rev 278459)
@@ -338,6 +338,12 @@
return _positionInformation;
}
+static bool isJavaScriptURL(NSURL *url)
+{
+ auto scheme = url.scheme;
+ return scheme && [scheme caseInsensitiveCompare:@"_javascript_"] == NSOrderedSame;
+}
+
- (void)_createSheetWithElementActions:(NSArray *)actions defaultTitle:(NSString *)defaultTitle showLinkTitle:(BOOL)showLinkTitle
{
auto delegate = _delegate.get();
@@ -348,8 +354,6 @@
return;
NSURL *targetURL = _positionInformation->url;
- NSString *urlScheme = [targetURL scheme];
- BOOL isJavaScriptURL = [urlScheme length] && [urlScheme caseInsensitiveCompare:@"_javascript_"] == NSOrderedSame;
// FIXME: We should check if _javascript_ is enabled in the preferences.
_interactionSheet = adoptNS([[WKActionSheet alloc] init]);
@@ -359,7 +363,7 @@
NSString *titleString = nil;
BOOL titleIsURL = NO;
if (showLinkTitle && [[targetURL absoluteString] length]) {
- if (isJavaScriptURL)
+ if (isJavaScriptURL(targetURL))
titleString = WEB_UI_STRING_KEY("_javascript_", "_javascript_ Action Sheet Title", "Title for action sheet for _javascript_ link");
else {
titleString = WTF::userVisibleString(targetURL);
@@ -563,7 +567,7 @@
[defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeSaveImage assistant:self]];
}
- if (![[targetURL scheme] length] || [[targetURL scheme] caseInsensitiveCompare:@"_javascript_"] != NSOrderedSame) {
+ if (!isJavaScriptURL(targetURL)) {
[defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeCopy assistant:self]];
[defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeShare assistant:self]];
}
@@ -597,9 +601,9 @@
#endif
if (TCCAccessPreflight(getkTCCServicePhotos(), NULL) != kTCCAccessPreflightDenied)
[defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeSaveImage assistant:self]];
- if (!targetURL.scheme.length || [targetURL.scheme caseInsensitiveCompare:@"_javascript_"] != NSOrderedSame)
- [defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeCopy assistant:self]];
+ [defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeCopy assistant:self]];
+
#if ENABLE(IMAGE_EXTRACTION)
if ([_delegate respondsToSelector:@selector(actionSheetAssistant:shouldIncludeImageExtractionActionForElement:)] && [_delegate actionSheetAssistant:self shouldIncludeImageExtractionActionForElement:elementInfo])
[defaultActions addObject:[_WKElementAction _elementActionWithType:_WKElementActionTypeImageExtraction assistant:self]];
@@ -1017,8 +1021,10 @@
case _WKElementActionTypeShare:
if (URL(element.imageURL).protocolIsData() && element.image && [delegate respondsToSelector:@selector(actionSheetAssistant:shareElementWithImage:rect:)])
[delegate actionSheetAssistant:self shareElementWithImage:element.image rect:element.boundingRect];
- else
- [delegate actionSheetAssistant:self shareElementWithURL:element.URL ?: element.imageURL rect:element.boundingRect];
+ else {
+ auto urlToShare = element.URL && !isJavaScriptURL(element.URL) ? element.URL : element.imageURL;
+ [delegate actionSheetAssistant:self shareElementWithURL:urlToShare rect:element.boundingRect];
+ }
break;
case _WKElementActionTypeImageExtraction:
#if ENABLE(IMAGE_EXTRACTION)
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (278458 => 278459)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-06-04 15:03:00 UTC (rev 278458)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-06-04 15:32:21 UTC (rev 278459)
@@ -10372,8 +10372,21 @@
return;
}
+ bool canShowHTTPLinkOrDataDetectorPreview = ([&] {
+ if (linkURL.protocolIsInHTTPFamily())
+ return true;
+
+ if (WebCore::DataDetection::canBePresentedByDataDetectors(linkURL))
+ return true;
+
+ if ([strongSelf positionInformationHasImageOverlayDataDetector])
+ return true;
+
+ return false;
+ })();
+
ASSERT_IMPLIES(strongSelf->_positionInformation.isImage, strongSelf->_positionInformation.image);
- if (strongSelf->_positionInformation.isImage && strongSelf->_positionInformation.image && !strongSelf->_positionInformation.isLink) {
+ if (strongSelf->_positionInformation.isImage && strongSelf->_positionInformation.image && !canShowHTTPLinkOrDataDetectorPreview) {
auto cgImage = strongSelf->_positionInformation.image->makeCGImageCopy();
strongSelf->_contextMenuActionProviderDelegateNeedsOverride = NO;
@@ -10414,7 +10427,7 @@
#if ENABLE(DATA_DETECTION)
// FIXME: Support _javascript_ urls here. But make sure they don't show a preview.
// <rdar://problem/50572283>
- if (!linkURL.protocolIsInHTTPFamily() && !WebCore::DataDetection::canBePresentedByDataDetectors(linkURL) && ![strongSelf positionInformationHasImageOverlayDataDetector]) {
+ if (!canShowHTTPLinkOrDataDetectorPreview) {
continueWithContextMenuConfiguration(nil);
return;
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (278458 => 278459)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-04 15:03:00 UTC (rev 278458)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-04 15:32:21 UTC (rev 278459)
@@ -3157,16 +3157,18 @@
if (static_cast<SheetAction>(action) == SheetAction::Copy) {
if (is<RenderImage>(*element.renderer())) {
- URL url;
- String title;
- if (auto* linkElement = containingLinkAnchorElement(element)) {
- url = ""
- title = linkElement->attributeWithoutSynchronization(HTMLNames::titleAttr);
- if (!title.length())
- title = linkElement->textContent();
- title = stripLeadingAndTrailingHTMLSpaces(title);
+ URL urlToCopy;
+ String titleToCopy;
+ if (auto linkElement = makeRefPtr(containingLinkAnchorElement(element))) {
+ if (auto url = "" !url.isEmpty() && !url.protocolIsJavaScript()) {
+ urlToCopy = url;
+ titleToCopy = linkElement->attributeWithoutSynchronization(HTMLNames::titleAttr);
+ if (!titleToCopy.length())
+ titleToCopy = linkElement->textContent();
+ titleToCopy = stripLeadingAndTrailingHTMLSpaces(titleToCopy);
+ }
}
- m_interactionNode->document().editor().writeImageToPasteboard(*Pasteboard::createForCopyAndPaste(PagePasteboardContext::create(element.document().pageID())), element, url, title);
+ m_interactionNode->document().editor().writeImageToPasteboard(*Pasteboard::createForCopyAndPaste(PagePasteboardContext::create(element.document().pageID())), element, urlToCopy, titleToCopy);
} else if (element.isLink())
m_interactionNode->document().editor().copyURL(element.document().completeURL(stripLeadingAndTrailingHTMLSpaces(element.attributeWithoutSynchronization(HTMLNames::hrefAttr))), element.textContent());
#if ENABLE(ATTACHMENT_ELEMENT)
