- Revision
- 274772
- Author
- [email protected]
- Date
- 2021-03-22 12:06:02 -0700 (Mon, 22 Mar 2021)
Log Message
[macOS] Context menu should account for image overlay content
https://bugs.webkit.org/show_bug.cgi?id=223518
<rdar://problem/75505210>
Reviewed by Devin Rousso.
Source/WebCore:
Make some adjustments to allow context menu items for text selection to show up when right clicking on text in
an image overlay.
Test: fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html
* editing/Editor.cpp:
(WebCore::Editor::performCutOrCopy):
Adjust this logic so that we only attempt to write plain text to the system pasteboard when copying text inside
image overlays (this matches the behavior of copying selected text in `textarea` elements and text fields).
* html/HTMLElement.cpp:
(WebCore::HTMLElement::hasImageOverlay const):
(WebCore::imageOverlayHost):
(WebCore::HTMLElement::isInsideImageOverlay):
Add a helper function to determine whether or not a given range in the DOM is inside an image overlay shadow
root. To avoid code duplication, pull out some logic to grab the image overlay's element host (if it exists) out
into a separate static helper function (`imageOverlayHost`), and use it here and also in `isImageOverlayText`.
(WebCore::HTMLElement::isImageOverlayText):
Move the UA shadow root check into `HTMLElement::hasImageOverlay`, so that `hasImageOverlay` can be invoked for
elements with non-UA shadow roots without hitting an assertion.
* html/HTMLElement.h:
* page/ContextMenuController.cpp:
(WebCore::ContextMenuController::populate):
Pull logic that determines whether or not we should show text-selection-related context menu items out into a
separate lambda function, so that it's easier to take advantage of early returns; then, in the case where we
have an image URL, additionally show selected text options if the selection is inside the image overlay.
LayoutTests:
Add a layout test to exercise copying via the context menu.
* fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu-expected.txt: Added.
* fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (274771 => 274772)
--- trunk/LayoutTests/ChangeLog 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/LayoutTests/ChangeLog 2021-03-22 19:06:02 UTC (rev 274772)
@@ -1,3 +1,16 @@
+2021-03-22 Wenson Hsieh <[email protected]>
+
+ [macOS] Context menu should account for image overlay content
+ https://bugs.webkit.org/show_bug.cgi?id=223518
+ <rdar://problem/75505210>
+
+ Reviewed by Devin Rousso.
+
+ Add a layout test to exercise copying via the context menu.
+
+ * fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu-expected.txt: Added.
+ * fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html: Added.
+
2021-03-22 Chris Gambrell <[email protected]>
[LayoutTests] Convert http/tests/workers convert PHP to Python
Added: trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu-expected.txt (0 => 274772)
--- trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu-expected.txt 2021-03-22 19:06:02 UTC (rev 274772)
@@ -0,0 +1,5 @@
+PASS editor.textContent is "foobar"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html (0 => 274772)
--- trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html (rev 0)
+++ trunk/LayoutTests/fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html 2021-03-22 19:06:02 UTC (rev 274772)
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+img {
+ position: absolute;
+ top: 0;
+ left: 0;
+}
+
+div[contenteditable] {
+ border: 1px solid tomato;
+}
+</style>
+</head>
+<body>
+<img src=""
+<div contenteditable></div>
+<script>
+addEventListener("load", () => {
+ let image = document.querySelector("img");
+ internals.installImageOverlay(image, [{
+ text : "foobar",
+ topLeft : new DOMPointReadOnly(0, 0),
+ topRight : new DOMPointReadOnly(1, 0),
+ bottomRight : new DOMPointReadOnly(1, 0.5),
+ bottomLeft : new DOMPointReadOnly(0, 0.5),
+ }]);
+
+ eventSender.mouseMoveTo(50, 150);
+
+ for (let item of eventSender.contextClick()) {
+ if (item.title === "Copy")
+ item.click();
+ }
+
+ editor = document.querySelector("div[contenteditable]");
+ editor.focus();
+ document.execCommand("Paste");
+
+ shouldBeEqualToString("editor.textContent", "foobar");
+ editor.remove();
+});
+</script>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (274771 => 274772)
--- trunk/Source/WebCore/ChangeLog 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/Source/WebCore/ChangeLog 2021-03-22 19:06:02 UTC (rev 274772)
@@ -1,3 +1,44 @@
+2021-03-22 Wenson Hsieh <[email protected]>
+
+ [macOS] Context menu should account for image overlay content
+ https://bugs.webkit.org/show_bug.cgi?id=223518
+ <rdar://problem/75505210>
+
+ Reviewed by Devin Rousso.
+
+ Make some adjustments to allow context menu items for text selection to show up when right clicking on text in
+ an image overlay.
+
+ Test: fast/images/image-extraction/mac/copy-image-overlay-text-with-context-menu.html
+
+ * editing/Editor.cpp:
+ (WebCore::Editor::performCutOrCopy):
+
+ Adjust this logic so that we only attempt to write plain text to the system pasteboard when copying text inside
+ image overlays (this matches the behavior of copying selected text in `textarea` elements and text fields).
+
+ * html/HTMLElement.cpp:
+ (WebCore::HTMLElement::hasImageOverlay const):
+ (WebCore::imageOverlayHost):
+ (WebCore::HTMLElement::isInsideImageOverlay):
+
+ Add a helper function to determine whether or not a given range in the DOM is inside an image overlay shadow
+ root. To avoid code duplication, pull out some logic to grab the image overlay's element host (if it exists) out
+ into a separate static helper function (`imageOverlayHost`), and use it here and also in `isImageOverlayText`.
+
+ (WebCore::HTMLElement::isImageOverlayText):
+
+ Move the UA shadow root check into `HTMLElement::hasImageOverlay`, so that `hasImageOverlay` can be invoked for
+ elements with non-UA shadow roots without hitting an assertion.
+
+ * html/HTMLElement.h:
+ * page/ContextMenuController.cpp:
+ (WebCore::ContextMenuController::populate):
+
+ Pull logic that determines whether or not we should show text-selection-related context menu items out into a
+ separate lambda function, so that it's easier to take advantage of early returns; then, in the case where we
+ have an image URL, additionally show selected text options if the selection is inside the image overlay.
+
2021-03-22 Sam Weinig <[email protected]>
Use the PropertyName parameter passed to custom getters/setters rather than a redundant const char* in DOM attribute prologues
Modified: trunk/Source/WebCore/editing/Editor.cpp (274771 => 274772)
--- trunk/Source/WebCore/editing/Editor.cpp 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/Source/WebCore/editing/Editor.cpp 2021-03-22 19:06:02 UTC (rev 274772)
@@ -1411,7 +1411,7 @@
updateMarkersForWordsAffectedByEditing(true);
}
- if (enclosingTextFormControl(m_document.selection().selection().start()))
+ if (enclosingTextFormControl(m_document.selection().selection().start()) || (selection && HTMLElement::isInsideImageOverlay(*selection)))
Pasteboard::createForCopyAndPaste(PagePasteboardContext::create(m_document.pageID()))->writePlainText(selectedTextForDataTransfer(), canSmartCopyOrDelete() ? Pasteboard::CanSmartReplace : Pasteboard::CannotSmartReplace);
else {
HTMLImageElement* imageElement = nullptr;
Modified: trunk/Source/WebCore/html/HTMLElement.cpp (274771 => 274772)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2021-03-22 19:06:02 UTC (rev 274772)
@@ -1227,26 +1227,45 @@
bool HTMLElement::hasImageOverlay() const
{
- auto shadowRoot = userAgentShadowRoot();
- if (LIKELY(!shadowRoot))
+ auto shadowRoot = this->shadowRoot();
+ if (LIKELY(!shadowRoot || shadowRoot->mode() != ShadowRootMode::UserAgent))
return false;
return shadowRoot->hasElementWithId(*imageOverlayElementIdentifier().impl());
}
-bool HTMLElement::isImageOverlayText(const Node& node)
+static RefPtr<HTMLElement> imageOverlayHost(const Node& node)
{
- auto shadowHost = node.shadowHost();
- if (!shadowHost)
+ auto host = node.shadowHost();
+ if (!is<HTMLElement>(host))
+ return nullptr;
+
+ auto element = makeRefPtr(downcast<HTMLElement>(*host));
+ return element->hasImageOverlay() ? element : nullptr;
+}
+
+bool HTMLElement::isInsideImageOverlay(const SimpleRange& range)
+{
+ auto commonAncestor = makeRefPtr(commonInclusiveAncestor<ComposedTree>(range));
+ if (!commonAncestor)
return false;
- auto shadowRoot = shadowHost->shadowRoot();
- if (!shadowRoot || shadowRoot->mode() != ShadowRootMode::UserAgent || shadowRoot != node.containingShadowRoot())
+ auto host = imageOverlayHost(*commonAncestor);
+ if (!host)
return false;
- for (auto& child : childrenOfType<HTMLDivElement>(*shadowRoot)) {
+ return host->userAgentShadowRoot()->contains(*commonAncestor);
+}
+
+bool HTMLElement::isImageOverlayText(const Node& node)
+{
+ auto host = imageOverlayHost(node);
+ if (!host)
+ return false;
+
+ for (auto& child : childrenOfType<HTMLDivElement>(*host->userAgentShadowRoot())) {
if (child.getIdAttribute() == imageOverlayElementIdentifier())
- return node.isDescendantOf(&child);
+ return node.isDescendantOf(child);
}
return false;
Modified: trunk/Source/WebCore/html/HTMLElement.h (274771 => 274772)
--- trunk/Source/WebCore/html/HTMLElement.h 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/Source/WebCore/html/HTMLElement.h 2021-03-22 19:06:02 UTC (rev 274772)
@@ -35,6 +35,7 @@
class FormNamedItem;
class HTMLFormElement;
class VisibleSelection;
+struct SimpleRange;
#if ENABLE(IMAGE_EXTRACTION)
struct ImageExtractionResult;
@@ -128,6 +129,7 @@
WEBCORE_EXPORT static bool shouldExtendSelectionToTargetNode(const Node& targetNode, const VisibleSelection& selectionBeforeUpdate);
bool hasImageOverlay() const;
+ static bool isInsideImageOverlay(const SimpleRange&);
WEBCORE_EXPORT static bool isImageOverlayText(const Node&);
#if ENABLE(IMAGE_EXTRACTION)
Modified: trunk/Source/WebCore/page/ContextMenuController.cpp (274771 => 274772)
--- trunk/Source/WebCore/page/ContextMenuController.cpp 2021-03-22 19:02:52 UTC (rev 274771)
+++ trunk/Source/WebCore/page/ContextMenuController.cpp 2021-03-22 19:06:02 UTC (rev 274772)
@@ -914,7 +914,25 @@
}
}
- if (imageURL.isEmpty() && linkURL.isEmpty() && mediaURL.isEmpty()) {
+ bool shouldShowItemsForNonEditableText = ([&] {
+ if (!linkURL.isEmpty())
+ return false;
+
+ if (!mediaURL.isEmpty())
+ return false;
+
+ if (!imageURL.isEmpty()) {
+ auto selectedRange = frame->selection().selection().range();
+ return selectedRange && HTMLElement::isInsideImageOverlay(*selectedRange);
+ }
+
+ return true;
+ })();
+
+ if (shouldShowItemsForNonEditableText) {
+ if (!imageURL.isEmpty())
+ appendItem(*separatorItem(), m_contextMenu.get());
+
if (m_context.hitTestResult().isSelected()) {
if (!selectedString.isEmpty()) {
#if PLATFORM(COCOA)
