Diff
Modified: trunk/LayoutTests/ChangeLog (276870 => 276871)
--- trunk/LayoutTests/ChangeLog 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/LayoutTests/ChangeLog 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1,3 +1,17 @@
+2021-04-30 Wenson Hsieh <[email protected]>
+
+ App highlight UI should be disabled when selecting text in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=225260
+ rdar://77359313
+
+ Reviewed by Tim Horton.
+
+ Add a new layout test to verify that the app higlight context menu items don't show up on macOS when right
+ clicking.
+
+ * fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items-expected.txt: Added.
+ * fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items.html: Added.
+
2021-04-30 Kyle Piddington <[email protected]>
[Metal ANGLE]: fix webgl/2.0.0/deqp/functional/gles3/negativebufferapi
Added: trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items-expected.txt (0 => 276871)
--- trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items-expected.txt 2021-05-01 03:26:53 UTC (rev 276871)
@@ -0,0 +1,9 @@
+PASS foundTitle1 is true
+PASS foundTitle2 is true
+PASS Selected image overlay
+PASS foundTitle1 is false
+PASS foundTitle2 is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items.html (0 => 276871)
--- trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items.html (rev 0)
+++ trunk/LayoutTests/fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items.html 2021-05-01 03:26:53 UTC (rev 276871)
@@ -0,0 +1,77 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ appHighlightsEnabled=true ] -->
+<html>
+<head>
+<script src=""
+<style>
+body, html {
+ margin: 0;
+}
+
+p {
+ font-size: 32px;
+ display: inline-block;
+}
+</style>
+</head>
+<body>
+<img src=""
+<p>Hello world this is a test.</p>
+</body>
+<script>
+var foundTitle1;
+var foundTitle2;
+
+function checkForAppHighlightContextMenuItems() {
+ foundTitle1 = false;
+ foundTitle2 = false;
+
+ let [title1, title2] = internals.appHighlightContextMenuItemTitles;
+ for (let item of eventSender.contextClick()) {
+ if (item.title === title1)
+ foundTitle1 = true;
+
+ if (item.title === title2)
+ foundTitle2 = true;
+ }
+}
+
+addEventListener("load", () => {
+ let text = document.querySelector("p");
+ let textRect = text.getBoundingClientRect();
+ let image = document.querySelector("img");
+
+ internals.installImageOverlay(image, [
+ {
+ topLeft : new DOMPointReadOnly(0, 0),
+ topRight : new DOMPointReadOnly(1, 0),
+ bottomRight : new DOMPointReadOnly(1, 1),
+ bottomLeft : new DOMPointReadOnly(0, 1),
+ children: [
+ {
+ text : "foobar",
+ topLeft : new DOMPointReadOnly(0, 0),
+ topRight : new DOMPointReadOnly(1, 0),
+ bottomRight : new DOMPointReadOnly(1, 1),
+ bottomLeft : new DOMPointReadOnly(0, 1),
+ }
+ ]
+ }
+ ]);
+
+ getSelection().selectAllChildren(text);
+ eventSender.mouseMoveTo(textRect.left + textRect.width / 2, textRect.top + textRect.height / 2);
+ checkForAppHighlightContextMenuItems();
+ shouldBeTrue("foundTitle1");
+ shouldBeTrue("foundTitle2");
+
+ getSelection().selectAllChildren(internals.shadowRoot(image).querySelector("div#image-overlay"));
+ testPassed("Selected image overlay")
+ eventSender.mouseMoveTo(200, 200);
+ checkForAppHighlightContextMenuItems();
+ shouldBeFalse("foundTitle1");
+ shouldBeFalse("foundTitle2");
+
+ text.remove();
+});
+</script>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (276870 => 276871)
--- trunk/Source/WebCore/ChangeLog 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebCore/ChangeLog 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1,3 +1,33 @@
+2021-04-30 Wenson Hsieh <[email protected]>
+
+ App highlight UI should be disabled when selecting text in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=225260
+ rdar://77359313
+
+ Reviewed by Tim Horton.
+
+ When both app highlights and image extraction are enabled, parts of the app highlights feature don't play well
+ with highlights created for text selections inside an image overlay. For the time being, address this by
+ disabling app highlights in image overlays.
+
+ These changes are exercised by a new API test on iOS, and a new layout test for macOS.
+
+ Tests: WebKit.AppHighlightsInImageOverlays
+ fast/images/image-extraction/mac/image-overlay-text-disables-app-highlight-menu-items.html
+
+ * page/ContextMenuController.cpp:
+ (WebCore::ContextMenuController::populate):
+
+ Don't add app highlight menu items in the case where we've selected text in an image overlay.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::appHighlightContextMenuItemTitles const):
+
+ Add an internal testing hook to expose the localized names of the app highlight menu items to layout tests.
+
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2021-04-30 Jer Noble <[email protected]>
[Cocoa] Calling into -[AVPlayerItem currentTime] is very expensive
Modified: trunk/Source/WebCore/page/ContextMenuController.cpp (276870 => 276871)
--- trunk/Source/WebCore/page/ContextMenuController.cpp 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebCore/page/ContextMenuController.cpp 2021-05-01 03:26:53 UTC (rev 276871)
@@ -935,6 +935,8 @@
}
}
+ auto selectedRange = frame->selection().selection().range();
+ bool selectionIsInsideImageOverlay = selectedRange && HTMLElement::isInsideImageOverlay(*selectedRange);
bool shouldShowItemsForNonEditableText = ([&] {
if (!linkURL.isEmpty())
return false;
@@ -942,10 +944,8 @@
if (!mediaURL.isEmpty())
return false;
- if (!imageURL.isEmpty()) {
- auto selectedRange = frame->selection().selection().range();
- return selectedRange && HTMLElement::isInsideImageOverlay(*selectedRange);
- }
+ if (!imageURL.isEmpty())
+ return selectionIsInsideImageOverlay;
return true;
})();
@@ -963,7 +963,7 @@
#if ENABLE(APP_HIGHLIGHTS)
if (auto* page = frame->page()) {
- if (page->settings().appHighlightsEnabled()) {
+ if (page->settings().appHighlightsEnabled() && !selectionIsInsideImageOverlay) {
appendItem(AddHighlightToNewGroupItem, m_contextMenu.get());
appendItem(AddHighlightItem, m_contextMenu.get());
appendItem(*separatorItem(), m_contextMenu.get());
Modified: trunk/Source/WebCore/testing/Internals.cpp (276870 => 276871)
--- trunk/Source/WebCore/testing/Internals.cpp 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebCore/testing/Internals.cpp 2021-05-01 03:26:53 UTC (rev 276871)
@@ -124,6 +124,7 @@
#include "LegacySchemeRegistry.h"
#include "LibWebRTCProvider.h"
#include "LoaderStrategy.h"
+#include "LocalizedStrings.h"
#include "Location.h"
#include "MallocStatistics.h"
#include "MediaDevices.h"
@@ -6053,6 +6054,14 @@
#endif
#if ENABLE(APP_HIGHLIGHTS)
+Vector<String> Internals::appHighlightContextMenuItemTitles() const
+{
+ return {{
+ contextMenuItemTagAddHighlightToCurrentGroup(),
+ contextMenuItemTagAddHighlightToNewGroup(),
+ }};
+}
+
unsigned Internals::numberOfAppHighlights()
{
Document* document = contextDocument();
Modified: trunk/Source/WebCore/testing/Internals.h (276870 => 276871)
--- trunk/Source/WebCore/testing/Internals.h 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebCore/testing/Internals.h 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1100,6 +1100,7 @@
bool destroySleepDisabler(unsigned identifier);
#if ENABLE(APP_HIGHLIGHTS)
+ Vector<String> appHighlightContextMenuItemTitles() const;
unsigned numberOfAppHighlights();
#endif
Modified: trunk/Source/WebCore/testing/Internals.idl (276870 => 276871)
--- trunk/Source/WebCore/testing/Internals.idl 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebCore/testing/Internals.idl 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1002,6 +1002,7 @@
unsigned long createSleepDisabler(DOMString reason, boolean display);
boolean destroySleepDisabler(unsigned long identifier);
+ [Conditional=APP_HIGHLIGHTS] readonly attribute sequence<DOMString> appHighlightContextMenuItemTitles;
[Conditional=APP_HIGHLIGHTS] unsigned long numberOfAppHighlights();
[Conditional=WEBXR] readonly attribute WebXRTest xrTest;
Modified: trunk/Source/WebKit/ChangeLog (276870 => 276871)
--- trunk/Source/WebKit/ChangeLog 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebKit/ChangeLog 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1,3 +1,34 @@
+2021-04-30 Wenson Hsieh <[email protected]>
+
+ App highlight UI should be disabled when selecting text in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=225260
+ rdar://77359313
+
+ Reviewed by Tim Horton.
+
+ Add a bit to `EditorState` to indicate when a ranged selection is inside an image overlay.
+
+ * Shared/EditorState.cpp:
+ (WebKit::EditorState::encode const):
+ (WebKit::EditorState::decode):
+ * Shared/EditorState.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView shouldAllowAppHighlightCreation]):
+ (-[WKContentView canPerformAction:withSender:]):
+ (-[WKContentView targetForAction:withSender:]):
+
+ Make `-targetForAction:withSender:` return `self` only if we have a ranged selection that is not inside an image
+ overlay. Interestingly, UIKit doesn't seem to consult `-canPerformAction:withSender:` for actions that are added
+ through `-[UIMenuController setMenuItems:]`, so we add the check here instead of inside `-canPerformAction:`
+ above.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::editorState const):
+
+ Set the new bit here. Note that this (importantly) doesn't trigger layout, because `VisibleSelection::range`
+ just returns a (potentially uncanonicalized) `SimpleRange`. For the purposes of checking whether the selection
+ is inside an image overlay, this is sufficient.
+
2021-04-30 Eric Carlson <[email protected]>
[GPUP] Poll videoPlaybackQualityMetrics at the frequency is it accessed by script
Modified: trunk/Source/WebKit/Shared/EditorState.cpp (276870 => 276871)
--- trunk/Source/WebKit/Shared/EditorState.cpp 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebKit/Shared/EditorState.cpp 2021-05-01 03:26:53 UTC (rev 276871)
@@ -39,6 +39,7 @@
encoder << shouldIgnoreSelectionChanges;
encoder << selectionIsNone;
encoder << selectionIsRange;
+ encoder << selectionIsRangeInsideImageOverlay;
encoder << isContentEditable;
encoder << isContentRichlyEditable;
encoder << isInPasswordField;
@@ -66,6 +67,9 @@
if (!decoder.decode(result.selectionIsRange))
return false;
+ if (!decoder.decode(result.selectionIsRangeInsideImageOverlay))
+ return false;
+
if (!decoder.decode(result.isContentEditable))
return false;
Modified: trunk/Source/WebKit/Shared/EditorState.h (276870 => 276871)
--- trunk/Source/WebKit/Shared/EditorState.h 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebKit/Shared/EditorState.h 2021-05-01 03:26:53 UTC (rev 276871)
@@ -75,6 +75,7 @@
bool shouldIgnoreSelectionChanges { false };
bool selectionIsNone { true }; // This will be false when there is a caret selection.
bool selectionIsRange { false };
+ bool selectionIsRangeInsideImageOverlay { false };
bool isContentEditable { false };
bool isContentRichlyEditable { false };
bool isInPasswordField { false };
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (276870 => 276871)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-05-01 03:26:53 UTC (rev 276871)
@@ -3548,6 +3548,16 @@
[_textInteractionAssistant activateSelection];
}
+#if ENABLE(APP_HIGHLIGHTS)
+
+- (BOOL)shouldAllowAppHighlightCreation
+{
+ auto editorState = _page->editorState();
+ return editorState.selectionIsRange && !editorState.isContentEditable && !editorState.selectionIsRangeInsideImageOverlay;
+}
+
+#endif // ENABLE(APP_HIGHLIGHTS)
+
- (BOOL)canPerformAction:(SEL)action withSender:(id)sender
{
if (_domPasteRequestHandler)
@@ -3566,8 +3576,11 @@
return editorState.isContentEditable;
#if ENABLE(APP_HIGHLIGHTS)
- if (action == @selector(createHighlightInCurrentGroupWithRange:) || action == @selector(createHighlightInNewGroupWithRange:))
- return editorState.selectionIsRange && !editorState.isContentEditable;
+ if (action == @selector(createHighlightInCurrentGroupWithRange:) || action == @selector(createHighlightInNewGroupWithRange:)) {
+ // FIXME: It doesn't seem like this codepath is exercised, since UIKit only asks for the action target for custom actions
+ // added via -[UIMenuController setMenuItems:]. Can we remove this check?
+ return self.shouldAllowAppHighlightCreation;
+ }
#endif
return [_webView canPerformAction:action withSender:sender];
@@ -3762,7 +3775,7 @@
{
#if ENABLE(APP_HIGHLIGHTS)
if (action == @selector(createHighlightInCurrentGroupWithRange:) || action == @selector(createHighlightInNewGroupWithRange:))
- return self;
+ return self.shouldAllowAppHighlightCreation ? self : nil;
#endif
return [_webView targetForAction:action withSender:sender];
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (276870 => 276871)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1208,6 +1208,11 @@
Ref<Document> document = *frame->document();
result.originIdentifierForPasteboard = document->originIdentifierForPasteboard();
+ if (result.selectionIsRange) {
+ auto selectionRange = selection.range();
+ result.selectionIsRangeInsideImageOverlay = selectionRange && HTMLElement::isInsideImageOverlay(*selectionRange);
+ }
+
if (shouldPerformLayout == ShouldPerformLayout::Yes || platformNeedsLayoutForEditorState(frame))
document->updateLayout(); // May cause document destruction
Modified: trunk/Tools/ChangeLog (276870 => 276871)
--- trunk/Tools/ChangeLog 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Tools/ChangeLog 2021-05-01 03:26:53 UTC (rev 276871)
@@ -1,3 +1,25 @@
+2021-04-30 Wenson Hsieh <[email protected]>
+
+ App highlight UI should be disabled when selecting text in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=225260
+ rdar://77359313
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm:
+
+ Add a new API test to verify the behavior of `-targetForAction:withSender:` in `WKContentView`.
+
+ * WebKitTestRunner/TestOptions.cpp:
+ (WTR::TestOptions::defaults):
+ (WTR::TestOptions::keyTypeMapping):
+ * WebKitTestRunner/TestOptions.h:
+ (WTR::TestOptions::appHighlightsEnabled const):
+ * WebKitTestRunner/cocoa/TestControllerCocoa.mm:
+ (WTR::TestController::platformCreateWebView):
+
+ Add support for a new `TestOption` that enables app highlights. See the new layout test for more information.
+
2021-04-30 Carlos Alberto Lopez Perez <[email protected]>
[tools] Make run-buildbot-test compatible with buildbot 2.10.5
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm (276870 => 276871)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm 2021-05-01 03:26:53 UTC (rev 276871)
@@ -32,6 +32,8 @@
#import "TestNavigationDelegate.h"
#import "TestWKWebView.h"
#import "UIKitSPI.h"
+#import "WKWebViewConfigurationExtras.h"
+#import <WebKit/WKWebViewConfigurationPrivate.h>
#import <WebKit/WebKit.h>
#import <wtf/RetainPtr.h>
@@ -71,4 +73,32 @@
[webView waitForNextPresentationUpdate];
}
-#endif
+#if ENABLE(IMAGE_EXTRACTION) && ENABLE(APP_HIGHLIGHTS)
+
+TEST(WebKit, AppHighlightsInImageOverlays)
+{
+ auto configuration = retainPtr([WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES]);
+ [configuration _setAppHighlightsEnabled:YES];
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ [webView synchronouslyLoadTestPageNamed:@"simple-image-overlay"];
+ [webView stringByEvaluatingJavaScript:@"selectImageOverlay()"];
+ [webView waitForNextPresentationUpdate];
+
+ auto createHighlightInCurrentGroupWithRangeSelector = NSSelectorFromString(@"createHighlightInCurrentGroupWithRange:");
+ auto createHighlightInNewGroupWithRangeSelector = NSSelectorFromString(@"createHighlightInNewGroupWithRange:");
+
+ auto contentView = [webView textInputContentView];
+ EXPECT_NULL([contentView targetForAction:createHighlightInCurrentGroupWithRangeSelector withSender:nil]);
+ EXPECT_NULL([contentView targetForAction:createHighlightInNewGroupWithRangeSelector withSender:nil]);
+
+ [webView synchronouslyLoadTestPageNamed:@"simple"];
+ [webView selectAll:nil];
+ [webView waitForNextPresentationUpdate];
+ EXPECT_EQ([contentView targetForAction:createHighlightInCurrentGroupWithRangeSelector withSender:nil], contentView);
+ EXPECT_EQ([contentView targetForAction:createHighlightInNewGroupWithRangeSelector withSender:nil], contentView);
+}
+
+#endif // ENABLE(IMAGE_EXTRACTION) && ENABLE(APP_HIGHLIGHTS)
+
+#endif // PLATFORM(IOS_FAMILY)
Modified: trunk/Tools/WebKitTestRunner/TestOptions.cpp (276870 => 276871)
--- trunk/Tools/WebKitTestRunner/TestOptions.cpp 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Tools/WebKitTestRunner/TestOptions.cpp 2021-05-01 03:26:53 UTC (rev 276871)
@@ -135,6 +135,7 @@
{ "enableInAppBrowserPrivacy", false },
{ "enableProcessSwapOnNavigation", true },
{ "enableProcessSwapOnWindowOpen", false },
+ { "appHighlightsEnabled", false },
{ "ignoreSynchronousMessagingTimeouts", false },
{ "ignoresViewportScaleLimits", false },
{ "isAppBoundWebView", false },
@@ -186,6 +187,7 @@
{ "enableInAppBrowserPrivacy", TestHeaderKeyType::BoolTestRunner },
{ "enableProcessSwapOnNavigation", TestHeaderKeyType::BoolTestRunner },
{ "enableProcessSwapOnWindowOpen", TestHeaderKeyType::BoolTestRunner },
+ { "appHighlightsEnabled", TestHeaderKeyType::BoolTestRunner },
{ "ignoreSynchronousMessagingTimeouts", TestHeaderKeyType::BoolTestRunner },
{ "ignoresViewportScaleLimits", TestHeaderKeyType::BoolTestRunner },
{ "isAppBoundWebView", TestHeaderKeyType::BoolTestRunner },
Modified: trunk/Tools/WebKitTestRunner/TestOptions.h (276870 => 276871)
--- trunk/Tools/WebKitTestRunner/TestOptions.h 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Tools/WebKitTestRunner/TestOptions.h 2021-05-01 03:26:53 UTC (rev 276871)
@@ -55,6 +55,7 @@
bool enableInAppBrowserPrivacy() const { return boolTestRunnerFeatureValue("enableInAppBrowserPrivacy"); }
bool enableProcessSwapOnNavigation() const { return boolTestRunnerFeatureValue("enableProcessSwapOnNavigation"); }
bool enableProcessSwapOnWindowOpen() const { return boolTestRunnerFeatureValue("enableProcessSwapOnWindowOpen"); }
+ bool appHighlightsEnabled() const { return boolTestRunnerFeatureValue("appHighlightsEnabled"); }
bool ignoreSynchronousMessagingTimeouts() const { return boolTestRunnerFeatureValue("ignoreSynchronousMessagingTimeouts"); }
bool ignoresViewportScaleLimits() const { return boolTestRunnerFeatureValue("ignoresViewportScaleLimits"); }
bool isAppBoundWebView() const { return boolTestRunnerFeatureValue("isAppBoundWebView"); }
Modified: trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm (276870 => 276871)
--- trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm 2021-05-01 02:11:51 UTC (rev 276870)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm 2021-05-01 03:26:53 UTC (rev 276871)
@@ -176,6 +176,7 @@
[copiedConfiguration setWebsiteDataStore:(WKWebsiteDataStore *)websiteDataStore()];
[copiedConfiguration _setAllowTopNavigationToDataURLs:options.allowTopNavigationToDataURLs()];
+ [copiedConfiguration _setAppHighlightsEnabled:options.appHighlightsEnabled()];
configureContentMode(copiedConfiguration.get(), options);
