Title: [293977] trunk
Revision
293977
Author
katherine_che...@apple.com
Date
2022-05-09 09:22:40 -0700 (Mon, 09 May 2022)

Log Message

Image controls menu button is not appearing for multi-page PDFs
https://bugs.webkit.org/show_bug.cgi?id=240120
rdar://86425721

Reviewed by Megan Gardner.

Source/WebCore:

Test: fast/attachment/attachment-image-controls-basic.html

Refactor image controls button code so it can also be used for PDF
attachments.

* dom/mac/ImageControlsMac.cpp:
(WebCore::ImageControlsMac::handleEvent):
(WebCore::ImageControlsMac::isImageMenuEnabled):
(WebCore::ImageControlsMac::updateImageControls):
(WebCore::ImageControlsMac::tryCreateImageControls):
(WebCore::ImageControlsMac::destroyImageControls):
(WebCore::ImageControlsMac::hasImageControls):
* dom/mac/ImageControlsMac.h:
Handle the PDF case when a click on the image controls button happens.
Move all image controls code to ImageControlsMac where it can be
shared by image and attachment elements.

* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::parseAttribute):
(WebCore::HTMLAttachmentElement::childShouldCreateRenderer const):
* html/HTMLAttachmentElement.h:
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::parseAttribute):
(WebCore::HTMLImageElement::didAttachRenderers):
(WebCore::HTMLImageElement::setAttachmentElement):
(WebCore::HTMLImageElement::updateImageControls): Deleted.
(WebCore::HTMLImageElement::tryCreateImageControls): Deleted.
(WebCore::HTMLImageElement::destroyImageControls): Deleted.
(WebCore::HTMLImageElement::hasImageControls const): Deleted.
* html/HTMLImageElement.h:
(WebCore::HTMLImageElement::isImageMenuEnabled const):
(WebCore::HTMLImageElement::setImageMenuEnabled):
(WebCore::HTMLImageElement::imageMenuEnabled const): Deleted.
* html/shadow/mac/imageControlsMac.css:
The image controls button was originally offset by 20px from the top
and the right. This causes the image controls button to appear in the
middle of a PDF attachment and be difficult to see. This change makes
it flush against the top right corner for both images and PDFs.

(button#image-controls-button):
* page/ChromeClient.h:
(WebCore::ChromeClient::handlePDFServiceClick):
* rendering/RenderAttachment.cpp:
(WebCore::RenderAttachment::RenderAttachment):
(WebCore::RenderAttachment::layout):
(WebCore::RenderAttachment::layoutShadowContent):
* rendering/RenderAttachment.h:
Add canHaveGeneratedChildren() and canHaveChildren() so that
we render the image controls button in the shadow tree.

* rendering/RenderImage.cpp:
(WebCore::RenderImage::RenderImage):
* testing/Internals.cpp:
(WebCore::Internals::hasImageControls const):

Source/WebKit:

Refactor image controls button code so it can also be used for PDF
attachments.

* Shared/ContextMenuContextData.cpp:
(WebKit::ContextMenuContextData::controlledDataIsEditable const):
We need to make sure PDF attachments are marked as editable so the
context menu gets properly populated.

* Shared/ContextMenuContextData.h:
(WebKit::ContextMenuContextData::ContextMenuContextData):
* UIProcess/API/Cocoa/APIAttachmentCocoa.mm:
(API::Attachment::enclosingImageNSData const):
Remove the image check so we return the PDF NSData in the attachment
case.

* UIProcess/mac/WebContextMenuProxyMac.mm:
(WebKit::WebContextMenuProxyMac::setupServicesMenu):
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::handlePDFServiceClick):
* WebProcess/WebCoreSupport/WebChromeClient.h:
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::handlePDFServiceClick):
Plumbing for handling a click on the image controls button for PDFs.

LayoutTests:

* TestExpectations:
* fast/attachment/attachment-image-controls-basic.html: Added.
* platform/mac-wk2/TestExpectations:
* platform/mac/fast/attachment/attachment-image-controls-basic-expected.txt: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (293976 => 293977)


--- trunk/LayoutTests/ChangeLog	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/LayoutTests/ChangeLog	2022-05-09 16:22:40 UTC (rev 293977)
@@ -1,3 +1,16 @@
+2022-05-09  Kate Cheney  <katherine_che...@apple.com>
+
+        Image controls menu button is not appearing for multi-page PDFs
+        https://bugs.webkit.org/show_bug.cgi?id=240120
+        rdar://86425721
+
+        Reviewed by Megan Gardner.
+
+        * TestExpectations:
+        * fast/attachment/attachment-image-controls-basic.html: Added.
+        * platform/mac-wk2/TestExpectations:
+        * platform/mac/fast/attachment/attachment-image-controls-basic-expected.txt: Added.
+
 2022-05-09  Ziran Sun  <z...@igalia.com>
 
         Make input placeholder line-height declaration !important

Modified: trunk/LayoutTests/TestExpectations (293976 => 293977)


--- trunk/LayoutTests/TestExpectations	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/LayoutTests/TestExpectations	2022-05-09 16:22:40 UTC (rev 293977)
@@ -5214,3 +5214,6 @@
 webkit.org/b/239550 fast/text/simple-line-hyphens-with-text-align.html [ ImageOnlyFailure ]
 webkit.org/b/239550 fast/text/simple-line-layout-hyphen-limit-after.html [ ImageOnlyFailure ]
 webkit.org/b/239550 fast/text/simple-line-layout-hyphen-limit-before.html [ ImageOnlyFailure ]
+
+# Image controls menu is mac only.
+fast/attachment/attachment-image-controls-basic.html [ Skip ]

Added: trunk/LayoutTests/fast/attachment/attachment-image-controls-basic.html (0 => 293977)


--- trunk/LayoutTests/fast/attachment/attachment-image-controls-basic.html	                        (rev 0)
+++ trunk/LayoutTests/fast/attachment/attachment-image-controls-basic.html	2022-05-09 16:22:40 UTC (rev 293977)
@@ -0,0 +1,13 @@
+<!DOCTYPE html><!-- webkit-test-runner [ AttachmentElementEnabled=true ] -->
+<html>
+<head>
+<script>
+    if (window.internals)
+        internals.settings.setImageControlsEnabled(true);
+</script>
+</head>
+<body>
+    <attachment id="test" type="application/pdf" src=""
+    <img src=""
+</body>
+</html>

Added: trunk/LayoutTests/platform/mac/fast/attachment/attachment-image-controls-basic-expected.txt (0 => 293977)


--- trunk/LayoutTests/platform/mac/fast/attachment/attachment-image-controls-basic-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/attachment/attachment-image-controls-basic-expected.txt	2022-05-09 16:22:40 UTC (rev 293977)
@@ -0,0 +1,14 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x88
+  RenderBlock {HTML} at (0,0) size 800x88
+    RenderBody {BODY} at (8,8) size 784x72
+      RenderAttachment {ATTACHMENT} at (0,0) size 60x60
+      RenderText {#text} at (60,54) size 4x18
+        text run at (60,54) width 4: " "
+      RenderImage {IMG} at (64,48) size 20x20
+      RenderText {#text} at (0,0) size 0x0
+layer at (8,8) size 60x60
+  RenderBlock (relative positioned) {DIV} at (0,0) size 60x60 [color=#00000000]
+layer at (38,10) size 28x26
+  RenderButton {BUTTON} at (30,2) size 28x26 [color=#000000D8] [bgcolor=#C0C0C0] [border: (2px outset #C0C0C0)]

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (293976 => 293977)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2022-05-09 16:22:40 UTC (rev 293977)
@@ -895,6 +895,9 @@
 # Skipped in general expectations since they only work on iOS and Mac, WK2.
 media/deactivate-audio-session.html [ Pass ]
 
+# Image controls menu is mac only.
+fast/attachment/attachment-image-controls-basic.html [ Pass ]
+
 webkit.org/b/177687 http/tests/inspector/network/resource-sizes-memory-cache.html [ Pass Failure ]
 
 webkit.org/b/178472 http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html [ Skip ]

Modified: trunk/Source/WebCore/ChangeLog (293976 => 293977)


--- trunk/Source/WebCore/ChangeLog	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/ChangeLog	2022-05-09 16:22:40 UTC (rev 293977)
@@ -1,3 +1,66 @@
+2022-05-09  Kate Cheney  <katherine_che...@apple.com>
+
+        Image controls menu button is not appearing for multi-page PDFs
+        https://bugs.webkit.org/show_bug.cgi?id=240120
+        rdar://86425721
+
+        Reviewed by Megan Gardner.
+
+        Test: fast/attachment/attachment-image-controls-basic.html
+
+        Refactor image controls button code so it can also be used for PDF
+        attachments.
+
+        * dom/mac/ImageControlsMac.cpp:
+        (WebCore::ImageControlsMac::handleEvent):
+        (WebCore::ImageControlsMac::isImageMenuEnabled):
+        (WebCore::ImageControlsMac::updateImageControls):
+        (WebCore::ImageControlsMac::tryCreateImageControls):
+        (WebCore::ImageControlsMac::destroyImageControls):
+        (WebCore::ImageControlsMac::hasImageControls):
+        * dom/mac/ImageControlsMac.h:
+        Handle the PDF case when a click on the image controls button happens.
+        Move all image controls code to ImageControlsMac where it can be
+        shared by image and attachment elements.
+
+        * html/HTMLAttachmentElement.cpp:
+        (WebCore::HTMLAttachmentElement::parseAttribute):
+        (WebCore::HTMLAttachmentElement::childShouldCreateRenderer const):
+        * html/HTMLAttachmentElement.h:
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::parseAttribute):
+        (WebCore::HTMLImageElement::didAttachRenderers):
+        (WebCore::HTMLImageElement::setAttachmentElement):
+        (WebCore::HTMLImageElement::updateImageControls): Deleted.
+        (WebCore::HTMLImageElement::tryCreateImageControls): Deleted.
+        (WebCore::HTMLImageElement::destroyImageControls): Deleted.
+        (WebCore::HTMLImageElement::hasImageControls const): Deleted.
+        * html/HTMLImageElement.h:
+        (WebCore::HTMLImageElement::isImageMenuEnabled const):
+        (WebCore::HTMLImageElement::setImageMenuEnabled):
+        (WebCore::HTMLImageElement::imageMenuEnabled const): Deleted.
+        * html/shadow/mac/imageControlsMac.css:
+        The image controls button was originally offset by 20px from the top
+        and the right. This causes the image controls button to appear in the
+        middle of a PDF attachment and be difficult to see. This change makes
+        it flush against the top right corner for both images and PDFs.
+
+        (button#image-controls-button):
+        * page/ChromeClient.h:
+        (WebCore::ChromeClient::handlePDFServiceClick):
+        * rendering/RenderAttachment.cpp:
+        (WebCore::RenderAttachment::RenderAttachment):
+        (WebCore::RenderAttachment::layout):
+        (WebCore::RenderAttachment::layoutShadowContent):
+        * rendering/RenderAttachment.h:
+        Add canHaveGeneratedChildren() and canHaveChildren() so that
+        we render the image controls button in the shadow tree.
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::RenderImage):
+        * testing/Internals.cpp:
+        (WebCore::Internals::hasImageControls const):
+
 2022-05-09  Ziran Sun  <z...@igalia.com>
 
         Make input placeholder line-height declaration !important

Modified: trunk/Source/WebCore/dom/mac/ImageControlsMac.cpp (293976 => 293977)


--- trunk/Source/WebCore/dom/mac/ImageControlsMac.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/dom/mac/ImageControlsMac.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -34,6 +34,7 @@
 #include "ElementRareData.h"
 #include "Event.h"
 #include "EventHandler.h"
+#include "EventLoop.h"
 #include "EventNames.h"
 #include "HTMLAttachmentElement.h"
 #include "HTMLButtonElement.h"
@@ -42,6 +43,7 @@
 #include "HTMLNames.h"
 #include "HTMLStyleElement.h"
 #include "MouseEvent.h"
+#include "RenderAttachment.h"
 #include "RenderImage.h"
 #include "ShadowRoot.h"
 #include "UserAgentStyleSheets.h"
@@ -141,14 +143,6 @@
     auto& node = downcast<Node>(*mouseEvent.target());
 
     if (ImageControlsMac::isImageControlsButtonElement(node)) {
-        RefPtr shadowHost = dynamicDowncast<HTMLImageElement>(node.shadowHost());
-        if (!shadowHost)
-            return false;
-
-        auto* image = imageFromImageElementNode(*shadowHost);
-        if (!image)
-            return false;
-
         Ref element = downcast<Element>(node);
         auto* renderer = element->renderer();
         if (!renderer)
@@ -159,7 +153,15 @@
             return false;
 
         auto point = view->contentsToWindow(renderer->absoluteBoundingBoxRect()).minXMaxYCorner();
-        page->chrome().client().handleImageServiceClick(point, *image, *shadowHost);
+
+        if (RefPtr shadowHost = dynamicDowncast<HTMLImageElement>(node.shadowHost())) {
+            auto* image = imageFromImageElementNode(*shadowHost);
+            if (!image)
+                return false;
+            page->chrome().client().handleImageServiceClick(point, *image, *shadowHost);
+        } else if (RefPtr shadowHost = dynamicDowncast<HTMLAttachmentElement>(node.shadowHost()))
+            page->chrome().client().handlePDFServiceClick(point, *shadowHost);
+
         event.setDefaultHandled();
         return true;
     }
@@ -166,6 +168,75 @@
     return false;
 }
 
+static bool isImageMenuEnabled(HTMLElement& element)
+{
+    if (RefPtr imageElement = dynamicDowncast<HTMLImageElement>(element))
+        return imageElement->isImageMenuEnabled();
+
+    if (RefPtr attachmentElement = dynamicDowncast<HTMLAttachmentElement>(element))
+        return attachmentElement->isImageMenuEnabled();
+
+    return false;
+}
+
+void updateImageControls(HTMLElement& element)
+{
+    // If this is an image element and it is inside a shadow tree then it is part of an image control.
+    if (element.isInShadowTree())
+        return;
+
+    if (!element.document().settings().imageControlsEnabled())
+        return;
+
+    element.document().eventLoop().queueTask(TaskSource::InternalAsyncTask, [weakElement = WeakPtr { element }] {
+        RefPtr protectedElement = weakElement.get();
+        if (!protectedElement)
+            return;
+        ASSERT(is<HTMLAttachmentElement>(*protectedElement) || is<HTMLImageElement>(*protectedElement));
+        bool imageMenuEnabled = isImageMenuEnabled(*protectedElement);
+        bool hasControls = hasImageControls(*protectedElement);
+        if (!imageMenuEnabled && hasControls)
+            destroyImageControls(*protectedElement);
+        else if (imageMenuEnabled && !hasControls)
+            tryCreateImageControls(*protectedElement);
+    });
+}
+
+void tryCreateImageControls(HTMLElement& element)
+{
+    ASSERT(isImageMenuEnabled(element));
+    ASSERT(!hasImageControls(element));
+    createImageControls(element);
+}
+
+void destroyImageControls(HTMLElement& element)
+{
+    auto shadowRoot = element.userAgentShadowRoot();
+    if (!shadowRoot)
+        return;
+
+    if (RefPtr node = shadowRoot->firstChild()) {
+        if (!is<HTMLElement>(*node))
+            return;
+        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ImageControlsMac::hasControls(downcast<HTMLElement>(*node)));
+        shadowRoot->removeChild(*node);
+    }
+
+    auto* renderObject = element.renderer();
+    if (!renderObject)
+        return;
+
+    if (auto* renderImage = dynamicDowncast<RenderImage>(*renderObject))
+        renderImage->setHasShadowControls(false);
+    else if (auto* renderAttachment = dynamicDowncast<RenderAttachment>(*renderObject))
+        renderAttachment->setHasShadowControls(false);
+}
+
+bool hasImageControls(const HTMLElement& element)
+{
+    return hasControls(element);
+}
+
 #endif // ENABLE(SERVICE_CONTROLS)
 
 } // namespace ImageControlsMac

Modified: trunk/Source/WebCore/dom/mac/ImageControlsMac.h (293976 => 293977)


--- trunk/Source/WebCore/dom/mac/ImageControlsMac.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/dom/mac/ImageControlsMac.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -42,7 +42,10 @@
 bool isInsideImageControls(const Node&);
 void createImageControls(HTMLElement&);
 bool handleEvent(HTMLElement&, Event&);
-
+void updateImageControls(HTMLElement&);
+void tryCreateImageControls(HTMLElement&);
+void destroyImageControls(HTMLElement&);
+WEBCORE_EXPORT bool hasImageControls(const HTMLElement&);
 #endif // ENABLE(SERVICE_CONTROLS)
 
 } // namespace ImageControlsMac

Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.cpp (293976 => 293977)


--- trunk/Source/WebCore/html/HTMLAttachmentElement.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -39,6 +39,7 @@
 #include "HTMLNames.h"
 #include "MIMETypeRegistry.h"
 #include "RenderAttachment.h"
+#include "ShadowRoot.h"
 #include "SharedBuffer.h"
 #include <pal/FileSizeFormatter.h>
 #include <wtf/IsoMallocInlines.h>
@@ -45,6 +46,10 @@
 #include <wtf/UUID.h>
 #include <wtf/URLParser.h>
 
+#if ENABLE(SERVICE_CONTROLS)
+#include "ImageControlsMac.h"
+#endif
+
 #if PLATFORM(COCOA)
 #include "UTIUtilities.h"
 #endif
@@ -169,6 +174,13 @@
     }
 
     HTMLElement::parseAttribute(name, value);
+
+#if ENABLE(SERVICE_CONTROLS)
+    if (name == typeAttr && attachmentType() == "application/pdf") {
+        setImageMenuEnabled(true);
+        ImageControlsMac::updateImageControls(*this);
+    }
+#endif
 }
 
 String HTMLAttachmentElement::attachmentTitle() const
@@ -282,6 +294,13 @@
     document().page()->attachmentElementClient()->requestAttachmentIcon(uniqueIdentifier(), size);
 }
 
+#if ENABLE(SERVICE_CONTROLS)
+bool HTMLAttachmentElement::childShouldCreateRenderer(const Node& child) const
+{
+    return hasShadowRootParent(child) && HTMLElement::childShouldCreateRenderer(child);
+}
+#endif
+
 } // namespace WebCore
 
 #endif // ENABLE(ATTACHMENT_ELEMENT)

Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.h (293976 => 293977)


--- trunk/Source/WebCore/html/HTMLAttachmentElement.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -77,6 +77,11 @@
     FloatSize iconSize() const { return m_iconSize; }
     RenderAttachment* renderer() const;
 
+#if ENABLE(SERVICE_CONTROLS)
+    bool isImageMenuEnabled() const { return m_isImageMenuEnabled; }
+    void setImageMenuEnabled(bool value) { m_isImageMenuEnabled = value; }
+#endif
+
 private:
     HTMLAttachmentElement(const QualifiedName&, Document&);
     virtual ~HTMLAttachmentElement();
@@ -91,12 +96,20 @@
     }
     bool canContainRangeEndPoint() const final { return false; }
     void parseAttribute(const QualifiedName&, const AtomString&) final;
-    
+
+#if ENABLE(SERVICE_CONTROLS)
+    bool childShouldCreateRenderer(const Node&) const final;
+#endif
+
     RefPtr<File> m_file;
     String m_uniqueIdentifier;
     RefPtr<Image> m_thumbnail;
     RefPtr<Image> m_icon;
     FloatSize m_iconSize;
+
+#if ENABLE(SERVICE_CONTROLS)
+    bool m_isImageMenuEnabled { false };
+#endif
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (293976 => 293977)


--- trunk/Source/WebCore/html/HTMLImageElement.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -328,8 +328,8 @@
         if (!parseCompositeAndBlendOperator(value, m_compositeOperator, blendOp))
             m_compositeOperator = CompositeOperator::SourceOver;
 #if ENABLE(SERVICE_CONTROLS)
-    } else if (m_imageMenuEnabled) {
-        updateImageControls();
+    } else if (isImageMenuEnabled()) {
+        ImageControlsMac::updateImageControls(*this);
 #endif
     } else if (name == loadingAttr) {
         // No action needed for eager to lazy transition.
@@ -400,7 +400,7 @@
         return;
 
 #if ENABLE(SERVICE_CONTROLS)
-    updateImageControls();
+    ImageControlsMac::updateImageControls(*this);
 #endif
 
     auto& renderImage = downcast<RenderImage>(*renderer());
@@ -758,7 +758,7 @@
     attachment->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
     ensureUserAgentShadowRoot().appendChild(WTFMove(attachment));
 #if ENABLE(SERVICE_CONTROLS)
-    m_imageMenuEnabled = true;
+    setImageMenuEnabled(true);
 #endif
 }
 
@@ -784,54 +784,11 @@
 #endif // ENABLE(ATTACHMENT_ELEMENT)
 
 #if ENABLE(SERVICE_CONTROLS)
-void HTMLImageElement::updateImageControls()
-{
-    // If this image element is inside a shadow tree then it is part of an image control.
-    if (isInShadowTree())
-        return;
-    if (!document().settings().imageControlsEnabled())
-        return;
-    document().eventLoop().queueTask(TaskSource::InternalAsyncTask, [this, protectedThis = Ref { *this }] {
-        bool hasControls = hasImageControls();
-        if (!m_imageMenuEnabled && hasControls)
-            destroyImageControls();
-        else if (m_imageMenuEnabled && !hasControls)
-            tryCreateImageControls();
-    });
-}
-
-void HTMLImageElement::tryCreateImageControls()
-{
-    ASSERT(m_imageMenuEnabled);
-    ASSERT(!hasImageControls());
-    ImageControlsMac::createImageControls(*this);
-}
-
-void HTMLImageElement::destroyImageControls()
-{
-    auto shadowRoot = userAgentShadowRoot();
-    if (!shadowRoot)
-        return;
-    if (RefPtr<Node> node = shadowRoot->firstChild()) {
-        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ImageControlsMac::hasControls(downcast<HTMLElement>(*node)));
-        shadowRoot->removeChild(*node);
-    }
-    auto* renderObject = renderer();
-    if (!renderObject)
-        return;
-    downcast<RenderImage>(*renderObject).setHasShadowControls(false);
-}
-
-bool HTMLImageElement::hasImageControls() const
-{
-    return ImageControlsMac::hasControls(*this);
-}
-
 bool HTMLImageElement::childShouldCreateRenderer(const Node& child) const
 {
     return hasShadowRootParent(child) && HTMLElement::childShouldCreateRenderer(child);
 }
-#endif // ENABLE(SERVICE_CONTROLS)
+#endif
 
 #if PLATFORM(IOS_FAMILY)
 // FIXME: We should find a better place for the touch callout logic. See rdar://problem/48937767.

Modified: trunk/Source/WebCore/html/HTMLImageElement.h (293976 => 293977)


--- trunk/Source/WebCore/html/HTMLImageElement.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/html/HTMLImageElement.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -117,7 +117,8 @@
     const AtomString& imageSourceURL() const override;
     
 #if ENABLE(SERVICE_CONTROLS)
-    bool imageMenuEnabled() const { return m_imageMenuEnabled; }
+    bool isImageMenuEnabled() const { return m_isImageMenuEnabled; }
+    void setImageMenuEnabled(bool value) { m_isImageMenuEnabled = value; }
 #endif
 
     HTMLPictureElement* pictureElement() const;
@@ -147,10 +148,6 @@
     ReferrerPolicy referrerPolicy() const;
 
     bool allowsOrientationOverride() const;
-    
-#if ENABLE(SERVICE_CONTROLS)
-    WEBCORE_EXPORT bool hasImageControls() const;
-#endif
 
 protected:
     HTMLImageElement(const QualifiedName&, Document&, HTMLFormElement* = nullptr);
@@ -204,9 +201,6 @@
     float effectiveImageDevicePixelRatio() const;
     
 #if ENABLE(SERVICE_CONTROLS)
-    void updateImageControls();
-    void tryCreateImageControls();
-    void destroyImageControls();
     bool childShouldCreateRenderer(const Node&) const override;
 #endif
 
@@ -223,7 +217,7 @@
     AtomString m_parsedUsemap;
     float m_imageDevicePixelRatio;
 #if ENABLE(SERVICE_CONTROLS)
-    bool m_imageMenuEnabled { false };
+    bool m_isImageMenuEnabled { false };
 #endif
     bool m_hadNameBeforeAttributeChanged { false }; // FIXME: We only need this because parseAttribute() can't see the old value.
     bool m_isDroppedImagePlaceholder { false };

Modified: trunk/Source/WebCore/html/shadow/mac/imageControlsMac.css (293976 => 293977)


--- trunk/Source/WebCore/html/shadow/mac/imageControlsMac.css	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/html/shadow/mac/imageControlsMac.css	2022-05-09 16:22:40 UTC (rev 293977)
@@ -36,9 +36,9 @@
 
 button#image-controls-button {
     position: absolute;
-    top: 20px;
-    right: 20px;
-    
+    top: 0px;
+    right: 0px;
+
     display: block;
 
     appearance: auto;

Modified: trunk/Source/WebCore/page/ChromeClient.h (293976 => 293977)


--- trunk/Source/WebCore/page/ChromeClient.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/page/ChromeClient.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -539,6 +539,7 @@
     virtual void handleSelectionServiceClick(FrameSelection&, const Vector<String>&, const IntPoint&) { }
     virtual bool hasRelevantSelectionServices(bool /*isTextOnly*/) const { return false; }
     virtual void handleImageServiceClick(const IntPoint&, Image&, HTMLImageElement&) { }
+    virtual void handlePDFServiceClick(const IntPoint&, HTMLAttachmentElement&) { };
 #endif
 
     virtual bool shouldDispatchFakeMouseMoveEvents() const { return true; }

Modified: trunk/Source/WebCore/rendering/RenderAttachment.cpp (293976 => 293977)


--- trunk/Source/WebCore/rendering/RenderAttachment.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/rendering/RenderAttachment.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -32,6 +32,7 @@
 #include "FloatRoundedRect.h"
 #include "FrameSelection.h"
 #include "HTMLAttachmentElement.h"
+#include "RenderChildIterator.h"
 #include "RenderTheme.h"
 #include <wtf/IsoMallocInlines.h>
 #include <wtf/URL.h>
@@ -45,6 +46,9 @@
 RenderAttachment::RenderAttachment(HTMLAttachmentElement& element, RenderStyle&& style)
     : RenderReplaced(element, WTFMove(style), LayoutSize())
 {
+#if ENABLE(SERVICE_CONTROLS)
+    m_hasShadowControls = element.isImageMenuEnabled();
+#endif
 }
 
 HTMLAttachmentElement& RenderAttachment::attachmentElement() const
@@ -64,6 +68,9 @@
     setIntrinsicSize(newIntrinsicSize);
 
     RenderReplaced::layout();
+    
+    if (hasShadowContent())
+        layoutShadowContent(newIntrinsicSize);
 }
 
 void RenderAttachment::invalidate()
@@ -96,6 +103,16 @@
     theme().paint(*this, controlStates, paintInfo, paintRect);
 }
 
+void RenderAttachment::layoutShadowContent(const LayoutSize& size)
+{
+    for (auto& renderBox : childrenOfType<RenderBox>(*this)) {
+        renderBox.mutableStyle().setHeight(Length(size.height(), LengthType::Fixed));
+        renderBox.mutableStyle().setWidth(Length(size.width(), LengthType::Fixed));
+        renderBox.setNeedsLayout(MarkOnlyThis);
+        renderBox.layout();
+    }
+}
+
 } // namespace WebCore
 
 #endif

Modified: trunk/Source/WebCore/rendering/RenderAttachment.h (293976 => 293977)


--- trunk/Source/WebCore/rendering/RenderAttachment.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/rendering/RenderAttachment.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -43,11 +43,16 @@
     bool shouldDrawBorder() const;
 
     void invalidate();
+    bool hasShadowContent() const { return m_hasShadowControls; }
+    void setHasShadowControls(bool hasShadowControls) { m_hasShadowControls = hasShadowControls; }
+    bool canHaveGeneratedChildren() const override { return m_hasShadowControls; }
+    bool canHaveChildren() const override { return m_hasShadowControls; }
 
 private:
     void element() const = delete;
     bool isAttachment() const override { return true; }
     ASCIILiteral renderName() const override { return "RenderAttachment"_s; }
+    void layoutShadowContent(const LayoutSize&);
 
     bool shouldDrawSelectionTint() const override { return false; }
     void paintReplaced(PaintInfo&, const LayoutPoint& offset) final;
@@ -58,6 +63,7 @@
 
     LayoutUnit m_minimumIntrinsicWidth;
     bool m_shouldDrawBorder { true };
+    bool m_hasShadowControls { false };
 };
 
 inline RenderAttachment* HTMLAttachmentElement::renderer() const

Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (293976 => 293977)


--- trunk/Source/WebCore/rendering/RenderImage.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -149,7 +149,7 @@
     updateAltText();
 #if ENABLE(SERVICE_CONTROLS)
     if (is<HTMLImageElement>(element))
-        m_hasShadowControls = downcast<HTMLImageElement>(element).imageMenuEnabled();
+        m_hasShadowControls = downcast<HTMLImageElement>(element).isImageMenuEnabled();
 #endif
 }
 

Modified: trunk/Source/WebCore/testing/Internals.cpp (293976 => 293977)


--- trunk/Source/WebCore/testing/Internals.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebCore/testing/Internals.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -375,6 +375,10 @@
 #include "HTMLModelElement.h"
 #endif
 
+#if ENABLE(SERVICE_CONTROLS)
+#include "ImageControlsMac.h"
+#endif
+
 using JSC::CallData;
 using JSC::CodeBlock;
 using JSC::FunctionExecutable;
@@ -6599,7 +6603,7 @@
 #if ENABLE(SERVICE_CONTROLS)
 bool Internals::hasImageControls(const HTMLImageElement& element) const
 {
-    return element.hasImageControls();
+    return ImageControlsMac::hasImageControls(element);
 }
 #endif
 

Modified: trunk/Source/WebKit/ChangeLog (293976 => 293977)


--- trunk/Source/WebKit/ChangeLog	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/ChangeLog	2022-05-09 16:22:40 UTC (rev 293977)
@@ -1,3 +1,36 @@
+2022-05-09  Kate Cheney  <katherine_che...@apple.com>
+
+        Image controls menu button is not appearing for multi-page PDFs
+        https://bugs.webkit.org/show_bug.cgi?id=240120
+        rdar://86425721
+
+        Reviewed by Megan Gardner.
+
+        Refactor image controls button code so it can also be used for PDF
+        attachments.
+
+        * Shared/ContextMenuContextData.cpp:
+        (WebKit::ContextMenuContextData::controlledDataIsEditable const):
+        We need to make sure PDF attachments are marked as editable so the
+        context menu gets properly populated.
+
+        * Shared/ContextMenuContextData.h:
+        (WebKit::ContextMenuContextData::ContextMenuContextData):
+        * UIProcess/API/Cocoa/APIAttachmentCocoa.mm:
+        (API::Attachment::enclosingImageNSData const):
+        Remove the image check so we return the PDF NSData in the attachment
+        case.
+
+        * UIProcess/mac/WebContextMenuProxyMac.mm:
+        (WebKit::WebContextMenuProxyMac::setupServicesMenu):
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::handlePDFServiceClick):
+        * WebProcess/WebCoreSupport/WebChromeClient.h:
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::handlePDFServiceClick):
+        Plumbing for handling a click on the image controls button for PDFs.
+
 2022-05-09  Diego Pino Garcia  <dp...@igalia.com>
 
         Unreviewed, non-unified build fixes after r293562

Modified: trunk/Source/WebKit/Shared/ContextMenuContextData.cpp (293976 => 293977)


--- trunk/Source/WebKit/Shared/ContextMenuContextData.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/Shared/ContextMenuContextData.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -160,7 +160,7 @@
 #if ENABLE(SERVICE_CONTROLS)
 bool ContextMenuContextData::controlledDataIsEditable() const
 {
-    if (!m_controlledSelectionData.isEmpty() || m_controlledImage)
+    if (!m_controlledSelectionData.isEmpty() || m_controlledImage || !m_controlledImageAttachmentID.isNull())
         return m_selectionIsEditable;
 
     return false;

Modified: trunk/Source/WebKit/Shared/ContextMenuContextData.h (293976 => 293977)


--- trunk/Source/WebKit/Shared/ContextMenuContextData.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/Shared/ContextMenuContextData.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -64,7 +64,17 @@
         , m_selectionIsEditable(isEditable)
     {
     }
-    
+
+    ContextMenuContextData(const WebCore::IntPoint& menuLocation, bool isEditable, const WebCore::IntRect& imageRect, const String& attachmentID, const String& sourceImageMIMEType)
+        : m_type(Type::ServicesMenu)
+        , m_menuLocation(menuLocation)
+        , m_selectionIsEditable(isEditable)
+        , m_controlledImageBounds(imageRect)
+        , m_controlledImageAttachmentID(attachmentID)
+        , m_controlledImageMIMEType(sourceImageMIMEType)
+    {
+    }
+
     ContextMenuContextData(const WebCore::IntPoint& menuLocation, WebCore::Image&, bool isEditable, const WebCore::IntRect& imageRect, const String& attachmentID, std::optional<WebCore::ElementContext>&&, const String& sourceImageMIMEType);
 
     ShareableBitmap* controlledImage() const { return m_controlledImage.get(); }

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm (293976 => 293977)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm	2022-05-09 16:22:40 UTC (rev 293977)
@@ -156,9 +156,6 @@
 
 NSData *Attachment::enclosingImageNSData() const
 {
-    if (!m_hasEnclosingImage)
-        return nil;
-
     auto fileWrapper = this->fileWrapper();
 
     if (![fileWrapper isRegularFile])

Modified: trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm (293976 => 293977)


--- trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm	2022-05-09 16:22:40 UTC (rev 293977)
@@ -215,10 +215,18 @@
 {
     bool includeEditorServices = m_context.controlledDataIsEditable();
     bool hasControlledImage = m_context.controlledImage();
+    bool isPDFAttachment = false;
+    auto attachment = page()->attachmentForIdentifier(m_context.controlledImageAttachmentID());
+    if (attachment) {
+#if HAVE(UNIFORM_TYPE_IDENTIFIERS_FRAMEWORK)
+        isPDFAttachment = attachment->utiType() == [UTTypePDF.identifier UTF8String];
+#else
+        isPDFAttachment = attachment->utiType() == String(kUTTypePDF);
+#endif
+    }
     NSArray *items = nil;
+    RetainPtr<NSItemProvider> itemProvider;
     if (hasControlledImage) {
-        auto attachment = page()->attachmentForIdentifier(m_context.controlledImageAttachmentID());
-        RetainPtr<NSItemProvider> itemProvider;
         if (attachment)
             itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:attachment->enclosingImageNSData() typeIdentifier:attachment->utiType()]);
         else {
@@ -239,6 +247,9 @@
         auto selection = adoptNS([[NSAttributedString alloc] initWithRTFD:selectionData.get() documentAttributes:nil]);
 
         items = @[ selection.get() ];
+    } else if (isPDFAttachment) {
+        itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:attachment->enclosingImageNSData() typeIdentifier:attachment->utiType()]);
+        items = @[ itemProvider.get() ];
     } else {
         LOG_ERROR("No service controlled item represented in the context");
         return;
@@ -245,7 +256,7 @@
     }
 
     RetainPtr<NSSharingServicePicker> picker = adoptNS([[NSSharingServicePicker alloc] initWithItems:items]);
-    [picker setStyle:hasControlledImage ? NSSharingServicePickerStyleRollover : NSSharingServicePickerStyleTextSelection];
+    [picker setStyle:hasControlledImage || isPDFAttachment ? NSSharingServicePickerStyleRollover : NSSharingServicePickerStyleTextSelection];
     [picker setDelegate:[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate]];
     [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setPicker:picker.get()];
     [[WKSharingServicePickerDelegate sharedSharingServicePickerDelegate] setFiltersEditingServices:!includeEditorServices];

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (293976 => 293977)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2022-05-09 16:22:40 UTC (rev 293977)
@@ -1367,6 +1367,11 @@
     m_page.handleImageServiceClick(point, image, element);
 }
 
+void WebChromeClient::handlePDFServiceClick(const IntPoint& point, HTMLAttachmentElement& element)
+{
+    m_page.handlePDFServiceClick(point, element);
+}
+
 #endif
 
 bool WebChromeClient::shouldDispatchFakeMouseMoveEvents() const

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h (293976 => 293977)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -384,6 +384,7 @@
     void handleSelectionServiceClick(WebCore::FrameSelection&, const Vector<String>& telephoneNumbers, const WebCore::IntPoint&) final;
     bool hasRelevantSelectionServices(bool isTextOnly) const final;
     void handleImageServiceClick(const WebCore::IntPoint&, WebCore::Image&, WebCore::HTMLImageElement&) final;
+    void handlePDFServiceClick(const WebCore::IntPoint&, WebCore::HTMLAttachmentElement&);
 #endif
 
     bool shouldDispatchFakeMouseMoveEvents() const final;

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (293976 => 293977)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2022-05-09 16:22:40 UTC (rev 293977)
@@ -1226,6 +1226,7 @@
     void handleTelephoneNumberClick(const String& number, const WebCore::IntPoint&, const WebCore::IntRect&);
     void handleSelectionServiceClick(WebCore::FrameSelection&, const Vector<String>& telephoneNumbers, const WebCore::IntPoint&);
     void handleImageServiceClick(const WebCore::IntPoint&, WebCore::Image&, WebCore::HTMLImageElement&);
+    void handlePDFServiceClick(const WebCore::IntPoint&, WebCore::HTMLAttachmentElement&);
 #endif
 
     void didChangeScrollOffsetForFrame(WebCore::Frame*);

Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm (293976 => 293977)


--- trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	2022-05-09 15:48:32 UTC (rev 293976)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	2022-05-09 16:22:40 UTC (rev 293977)
@@ -786,6 +786,17 @@
     }, { }));
 }
 
+void WebPage::handlePDFServiceClick(const IntPoint& point, HTMLAttachmentElement& element)
+{
+    send(Messages::WebPageProxy::ShowContextMenu(ContextMenuContextData {
+        point,
+        element.isContentEditable(),
+        element.renderBox()->absoluteContentQuad().enclosingBoundingBox(),
+        element.uniqueIdentifier(),
+        "application/pdf"_s
+    }, { }));
+}
+
 #endif
 
 String WebPage::platformUserAgent(const URL&) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to