[Attachment Support] Attachment elements don't appear in drag images on macOS
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2018 17:20:37 +0000 (17:20 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2018 17:20:37 +0000 (17:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188823
<rdar://problem/43616378>

Reviewed by Tim Horton.

Source/WebCore:

Currently, attachment elements don't show up in the drag image snapshot on macOS. This is because only the
"Selection" phase is painted when generating a drag image on macOS, and many replaced renderers (with some
exceptions, such as RenderImage) only paint visible content during the "Foreground" phase. To fix this, we
override RenderAttachment::paintReplaced to paint the attachment in the case where the Selection phase is being
painted.

Tests:  WKAttachmentTestsMac.DragAttachmentAsFilePromise
        WKAttachmentTests.MoveAttachmentElementAsIconByDragging

* rendering/RenderAttachment.cpp:
(WebCore::RenderAttachment::paintReplaced):
* rendering/RenderAttachment.h:
* rendering/RenderThemeMac.mm:
(WebCore::titleTextColorForAttachment):
(WebCore::AttachmentLayout::layOutTitle):

Plumb an AttachmentLayoutStyle (i.e. NonSelected or Selected) to AttachmentLayout, and use this bit when
determining the title text color, as well whether to paint backgrounds for the icon and title.

(WebCore::AttachmentLayout::AttachmentLayout):
(WebCore::RenderThemeMac::attachmentIntrinsicSize const):
(WebCore::RenderThemeMac::attachmentBaseline const):
(WebCore::paintAttachmentIconBackground):
(WebCore::paintAttachmentTitleBackground):

Bail from painting backgrounds if a selected style is used for the attachment.

(WebCore::RenderThemeMac::paintAttachment):

Rather than check the RenderAttachment's selection state when determining whether to paint with a non-selected
or selected style, only use selected style if the RenderAttachment has a selection _and_ the painting phase is
not "Selection". While this sounds extremely counter-intuitive, the "Selection" painting phase refers to
painting the selected foreground content _without_ including any part of the selection highlight.

Tools:

Adjusts a couple of existing tests to additionally verify that the drag image generated when dragging an
attachment element in macOS is not completely transparent.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(isCompletelyTransparent):
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235229 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderAttachment.cpp
Source/WebCore/rendering/RenderAttachment.h
Source/WebCore/rendering/RenderThemeMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm

index f2d9feb..05f2ae2 100644 (file)
@@ -1,3 +1,45 @@
+2018-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Attachment elements don't appear in drag images on macOS
+        https://bugs.webkit.org/show_bug.cgi?id=188823
+        <rdar://problem/43616378>
+
+        Reviewed by Tim Horton.
+
+        Currently, attachment elements don't show up in the drag image snapshot on macOS. This is because only the
+        "Selection" phase is painted when generating a drag image on macOS, and many replaced renderers (with some
+        exceptions, such as RenderImage) only paint visible content during the "Foreground" phase. To fix this, we
+        override RenderAttachment::paintReplaced to paint the attachment in the case where the Selection phase is being
+        painted.
+
+        Tests:  WKAttachmentTestsMac.DragAttachmentAsFilePromise
+                WKAttachmentTests.MoveAttachmentElementAsIconByDragging
+
+        * rendering/RenderAttachment.cpp:
+        (WebCore::RenderAttachment::paintReplaced):
+        * rendering/RenderAttachment.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::titleTextColorForAttachment):
+        (WebCore::AttachmentLayout::layOutTitle):
+
+        Plumb an AttachmentLayoutStyle (i.e. NonSelected or Selected) to AttachmentLayout, and use this bit when
+        determining the title text color, as well whether to paint backgrounds for the icon and title.
+
+        (WebCore::AttachmentLayout::AttachmentLayout):
+        (WebCore::RenderThemeMac::attachmentIntrinsicSize const):
+        (WebCore::RenderThemeMac::attachmentBaseline const):
+        (WebCore::paintAttachmentIconBackground):
+        (WebCore::paintAttachmentTitleBackground):
+
+        Bail from painting backgrounds if a selected style is used for the attachment.
+
+        (WebCore::RenderThemeMac::paintAttachment):
+
+        Rather than check the RenderAttachment's selection state when determining whether to paint with a non-selected
+        or selected style, only use selected style if the RenderAttachment has a selection _and_ the painting phase is
+        not "Selection". While this sounds extremely counter-intuitive, the "Selection" painting phase refers to
+        painting the selected foreground content _without_ including any part of the selection highlight.
+
 2018-08-23  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Floating] Decouple the incoming floats and floats already placed in the list
index a69fc93..ed3335e 100644 (file)
@@ -80,6 +80,18 @@ bool RenderAttachment::shouldDrawBorder() const
     return m_shouldDrawBorder;
 }
 
+void RenderAttachment::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& offset)
+{
+    if (paintInfo.phase != PaintPhase::Selection || !hasVisibleBoxDecorations() || !style().hasAppearance())
+        return;
+
+    auto paintRect = borderBoxRect();
+    paintRect.moveBy(offset);
+
+    ControlStates controlStates;
+    theme().paint(*this, controlStates, paintInfo, paintRect);
+}
+
 } // namespace WebCore
 
 #endif
index 4caae03..d32922a 100644 (file)
@@ -50,6 +50,7 @@ private:
     const char* renderName() const override { return "RenderAttachment"; }
 
     bool shouldDrawSelectionTint() const override { return false; }
+    void paintReplaced(PaintInfo&, const LayoutPoint& offset) final;
 
     void layout() override;
 
index b889dac..1009c21 100644 (file)
@@ -2544,8 +2544,10 @@ const CGFloat attachmentPlaceholderBorderDashLength = 6;
 
 const CGFloat attachmentMargin = 3;
 
+enum class AttachmentLayoutStyle : uint8_t { NonSelected, Selected };
+
 struct AttachmentLayout {
-    explicit AttachmentLayout(const RenderAttachment&);
+    explicit AttachmentLayout(const RenderAttachment&, AttachmentLayoutStyle);
 
     struct LabelLine {
         FloatRect backgroundRect;
@@ -2560,6 +2562,7 @@ struct AttachmentLayout {
     FloatRect attachmentRect;
 
     int baseline;
+    AttachmentLayoutStyle style;
 
     RetainPtr<CTLineRef> subtitleLine;
     FloatRect subtitleTextRect;
@@ -2571,11 +2574,11 @@ private:
     void addTitleLine(CTLineRef, CGFloat& yOffset, Vector<CGPoint> origins, CFIndex lineIndex, const RenderAttachment&);
 };
 
-static Color titleTextColorForAttachment(const RenderAttachment& attachment)
+static Color titleTextColorForAttachment(const RenderAttachment& attachment, AttachmentLayoutStyle style)
 {
     Color result = Color::black;
     
-    if (attachment.selectionState() != RenderObject::SelectionNone) {
+    if (style == AttachmentLayoutStyle::Selected) {
         if (attachment.frame().selection().isFocusedAndActive())
             result = colorFromNSColor([NSColor alternateSelectedControlTextColor]);
         else
@@ -2631,7 +2634,7 @@ void AttachmentLayout::layOutTitle(const RenderAttachment& attachment)
 
     NSDictionary *textAttributes = @{
         (__bridge id)kCTFontAttributeName: (__bridge id)font.get(),
-        (__bridge id)kCTForegroundColorAttributeName: (__bridge NSColor *)cachedCGColor(titleTextColorForAttachment(attachment))
+        (__bridge id)kCTForegroundColorAttributeName: (__bridge NSColor *)cachedCGColor(titleTextColorForAttachment(attachment, style))
     };
     RetainPtr<NSAttributedString> attributedTitle = adoptNS([[NSAttributedString alloc] initWithString:title attributes:textAttributes]);
     RetainPtr<CTFramesetterRef> titleFramesetter = adoptCF(CTFramesetterCreateWithAttributedString((CFAttributedStringRef)attributedTitle.get()));
@@ -2709,7 +2712,8 @@ void AttachmentLayout::layOutSubtitle(const RenderAttachment& attachment)
     subtitleTextRect = FloatRect(xOffset, yOffset, lineBounds.size.width, lineBounds.size.height);
 }
 
-AttachmentLayout::AttachmentLayout(const RenderAttachment& attachment)
+AttachmentLayout::AttachmentLayout(const RenderAttachment& attachment, AttachmentLayoutStyle layoutStyle)
+    : style(layoutStyle)
 {
     layOutTitle(attachment);
     layOutSubtitle(attachment);
@@ -2734,18 +2738,21 @@ AttachmentLayout::AttachmentLayout(const RenderAttachment& attachment)
 
 LayoutSize RenderThemeMac::attachmentIntrinsicSize(const RenderAttachment& attachment) const
 {
-    AttachmentLayout layout(attachment);
+    AttachmentLayout layout(attachment, AttachmentLayoutStyle::NonSelected);
     return LayoutSize(layout.attachmentRect.size());
 }
 
 int RenderThemeMac::attachmentBaseline(const RenderAttachment& attachment) const
 {
-    AttachmentLayout layout(attachment);
+    AttachmentLayout layout(attachment, AttachmentLayoutStyle::NonSelected);
     return layout.baseline;
 }
 
 static void paintAttachmentIconBackground(const RenderAttachment& attachment, GraphicsContext& context, AttachmentLayout& layout)
 {
+    if (layout.style == AttachmentLayoutStyle::NonSelected)
+        return;
+
     // FIXME: Finder has a discontinuous behavior here when you have a background color other than white,
     // where it switches into 'bordered mode' and the border pops in on top of the background.
     bool paintBorder = true;
@@ -2834,6 +2841,9 @@ static void paintAttachmentIconPlaceholder(const RenderAttachment& attachment, G
 
 static void paintAttachmentTitleBackground(const RenderAttachment& attachment, GraphicsContext& context, AttachmentLayout& layout)
 {
+    if (layout.style == AttachmentLayoutStyle::NonSelected)
+        return;
+
     if (layout.lines.isEmpty())
         return;
 
@@ -2932,7 +2942,11 @@ bool RenderThemeMac::paintAttachment(const RenderObject& renderer, const PaintIn
 
     const RenderAttachment& attachment = downcast<RenderAttachment>(renderer);
 
-    AttachmentLayout layout(attachment);
+    auto layoutStyle = AttachmentLayoutStyle::NonSelected;
+    if (attachment.selectionState() != RenderObject::SelectionNone && paintInfo.phase != PaintPhase::Selection)
+        layoutStyle = AttachmentLayoutStyle::Selected;
+
+    AttachmentLayout layout(attachment, layoutStyle);
 
     auto& progressString = attachment.attachmentElement().attributeWithoutSynchronization(progressAttr);
     bool validProgress = false;
@@ -2947,18 +2961,15 @@ bool RenderThemeMac::paintAttachment(const RenderObject& renderer, const PaintIn
     context.translate(toFloatSize(paintRect.location()));
     context.translate(floorSizeToDevicePixels(LayoutSize((paintRect.width() - attachmentIconBackgroundSize) / 2, 0), renderer.document().deviceScaleFactor()));
 
-    bool useSelectedStyle = attachment.selectionState() != RenderObject::SelectionNone;
     bool usePlaceholder = validProgress && !progress;
 
-    if (useSelectedStyle)
-        paintAttachmentIconBackground(attachment, context, layout);
+    paintAttachmentIconBackground(attachment, context, layout);
     if (usePlaceholder)
         paintAttachmentIconPlaceholder(attachment, context, layout);
     else
         paintAttachmentIcon(attachment, context, layout);
 
-    if (useSelectedStyle)
-        paintAttachmentTitleBackground(attachment, context, layout);
+    paintAttachmentTitleBackground(attachment, context, layout);
     paintAttachmentTitle(attachment, context, layout);
     paintAttachmentSubtitle(attachment, context, layout);
 
index 8b146b7..19e66f3 100644 (file)
@@ -1,3 +1,18 @@
+2018-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Attachment Support] Attachment elements don't appear in drag images on macOS
+        https://bugs.webkit.org/show_bug.cgi?id=188823
+        <rdar://problem/43616378>
+
+        Reviewed by Tim Horton.
+
+        Adjusts a couple of existing tests to additionally verify that the drag image generated when dragging an
+        attachment element in macOS is not completely transparent.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (isCompletelyTransparent):
+        (TestWebKitAPI::TEST):
+
 2018-08-23  Jonathan Bedard  <jbedard@apple.com>
 
         Explain test name matching in run-api-tests help
index fb8d515..8aaa1da 100644 (file)
@@ -394,6 +394,22 @@ static NSData *testPDFData()
 
 #pragma mark - Platform testing helper functions
 
+#if PLATFORM(MAC)
+
+BOOL isCompletelyTransparent(NSImage *image)
+{
+    auto representation = adoptNS([[NSBitmapImageRep alloc] initWithData:image.TIFFRepresentation]);
+    for (int row = 0; row < image.size.height; ++row) {
+        for (int column = 0; column < image.size.width; ++column) {
+            if ([representation colorAtX:column y:row].alphaComponent)
+                return false;
+        }
+    }
+    return true;
+}
+
+#endif
+
 #if PLATFORM(IOS)
 
 typedef void(^ItemProviderDataLoadHandler)(NSData *, NSError *);
@@ -1112,6 +1128,9 @@ TEST(WKAttachmentTests, MoveAttachmentElementAsIconByDragging)
     [attachment expectRequestedDataToBe:data.get()];
     EXPECT_WK_STREQ("document.pdf", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
     EXPECT_WK_STREQ("application/pdf", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+#if PLATFORM(MAC)
+    EXPECT_FALSE(isCompletelyTransparent([simulator draggingInfo].draggedImage));
+#endif
 
     [webView expectElementTag:@"STRONG" toComeBefore:@"ATTACHMENT"];
     [simulator endDataTransfer];
@@ -1210,6 +1229,7 @@ TEST(WKAttachmentTestsMac, DragAttachmentAsFilePromise)
     NSArray<NSURL *> *urls = [simulator receivePromisedFiles];
     EXPECT_EQ(1U, urls.count);
     EXPECT_TRUE([[NSData dataWithContentsOfURL:urls.firstObject] isEqualToData:testPDFData()]);
+    EXPECT_FALSE(isCompletelyTransparent([simulator draggingInfo].draggedImage));
 }
 
 #endif // PLATFORM(MAC)