Invoking a link preview on a complex link (e.g. an image) results in an empty TextInd...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Mar 2016 05:46:00 +0000 (05:46 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Mar 2016 05:46:00 +0000 (05:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155779
<rdar://problem/22408793>

Reviewed by Simon Fraser.

* page/FrameSnapshotting.cpp:
(WebCore::snapshotFrameRect):
(WebCore::snapshotFrameRectWithClip):
* page/FrameSnapshotting.h:
* page/TextIndicator.cpp:
(WebCore::takeSnapshot):
(WebCore::takeSnapshots):
(WebCore::initializeIndicator):
When snapshotting, clip to the indicated range's rects. This is important
to avoid painting into the margins in the non-selection-only painting case.
This didn't come up with normal selection-only painting because the text
didn't intersect the margin, and the background doesn't paint.

* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::dictionaryPopupInfoForRange):
(WebKit::WebPage::performImmediateActionHitTestAtLocation):
Use the TextIndicator mode where we give up on selection-only snapshotting
and just paint all content on Mac, similar to what we do for 3D Touch indicators.

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameSnapshotting.cpp
Source/WebCore/page/FrameSnapshotting.h
Source/WebCore/page/TextIndicator.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

index 7f4ee2a..c58bdac 100644 (file)
@@ -1,3 +1,24 @@
+2016-03-22  Tim Horton  <timothy_horton@apple.com>
+
+        Invoking a link preview on a complex link (e.g. an image) results in an empty TextIndicator
+        https://bugs.webkit.org/show_bug.cgi?id=155779
+        <rdar://problem/22408793>
+
+        Reviewed by Simon Fraser.
+
+        * page/FrameSnapshotting.cpp:
+        (WebCore::snapshotFrameRect):
+        (WebCore::snapshotFrameRectWithClip):
+        * page/FrameSnapshotting.h:
+        * page/TextIndicator.cpp:
+        (WebCore::takeSnapshot):
+        (WebCore::takeSnapshots):
+        (WebCore::initializeIndicator):
+        When snapshotting, clip to the indicated range's rects. This is important
+        to avoid painting into the margins in the non-selection-only painting case.
+        This didn't come up with normal selection-only painting because the text
+        didn't intersect the margin, and the background doesn't paint.
+
 2016-03-22  Darin Adler  <darin@apple.com>
 
         showModalDialog code runs with "first window" set to wrong window
index 4714dc6..3d34cd9 100644 (file)
@@ -32,6 +32,7 @@
 #include "FrameSnapshotting.h"
 
 #include "Document.h"
+#include "FloatRect.h"
 #include "Frame.h"
 #include "FrameSelection.h"
 #include "FrameView.h"
@@ -68,6 +69,12 @@ struct ScopedFramePaintingState {
 
 std::unique_ptr<ImageBuffer> snapshotFrameRect(Frame& frame, const IntRect& imageRect, SnapshotOptions options)
 {
+    Vector<FloatRect> clipRects;
+    return snapshotFrameRectWithClip(frame, imageRect, clipRects, options);
+}
+
+std::unique_ptr<ImageBuffer> snapshotFrameRectWithClip(Frame& frame, const IntRect& imageRect, Vector<FloatRect>& clipRects, SnapshotOptions options)
+{
     if (!frame.page())
         return nullptr;
 
@@ -104,6 +111,13 @@ std::unique_ptr<ImageBuffer> snapshotFrameRect(Frame& frame, const IntRect& imag
         return nullptr;
     buffer->context().translate(-imageRect.x(), -imageRect.y());
 
+    if (!clipRects.isEmpty()) {
+        Path clipPath;
+        for (auto& rect : clipRects)
+            clipPath.addRect(rect);
+        buffer->context().clipPath(clipPath);
+    }
+
     frame.view()->paintContentsForSnapshot(buffer->context(), imageRect, shouldIncludeSelection, coordinateSpace);
     return buffer;
 }
index 6f2d15e..c652d29 100644 (file)
 #define FrameSnapshotting_h
 
 #include <memory>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
+class FloatRect;
 class Frame;
 class IntRect;
 class ImageBuffer;
@@ -50,6 +52,7 @@ enum {
 typedef unsigned SnapshotOptions;
 
 WEBCORE_EXPORT std::unique_ptr<ImageBuffer> snapshotFrameRect(Frame&, const IntRect&, SnapshotOptions = SnapshotOptionsNone);
+std::unique_ptr<ImageBuffer> snapshotFrameRectWithClip(Frame&, const IntRect&, Vector<FloatRect>& clipRects, SnapshotOptions = SnapshotOptionsNone);
 std::unique_ptr<ImageBuffer> snapshotNode(Frame&, Node&);
 WEBCORE_EXPORT std::unique_ptr<ImageBuffer> snapshotSelection(Frame&, SnapshotOptions = SnapshotOptionsNone);
 
index fd0c0ba..17684c5 100644 (file)
@@ -149,26 +149,26 @@ static SnapshotOptions snapshotOptionsForTextIndicatorOptions(TextIndicatorOptio
     return snapshotOptions;
 }
 
-static RefPtr<Image> takeSnapshot(Frame& frame, IntRect rect, SnapshotOptions options, float& scaleFactor)
+static RefPtr<Image> takeSnapshot(Frame& frame, IntRect rect, SnapshotOptions options, float& scaleFactor, Vector<FloatRect>& clipRectsInDocumentCoordinates)
 {
-    std::unique_ptr<ImageBuffer> buffer = snapshotFrameRect(frame, rect, options);
+    std::unique_ptr<ImageBuffer> buffer = snapshotFrameRectWithClip(frame, rect, clipRectsInDocumentCoordinates, options);
     if (!buffer)
         return nullptr;
     scaleFactor = buffer->resolutionScale();
     return ImageBuffer::sinkIntoImage(WTFMove(buffer), Unscaled);
 }
 
-static bool takeSnapshots(TextIndicatorData& data, Frame& frame, IntRect snapshotRect)
+static bool takeSnapshots(TextIndicatorData& data, Frame& frame, IntRect snapshotRect, Vector<FloatRect>& clipRectsInDocumentCoordinates)
 {
     SnapshotOptions snapshotOptions = snapshotOptionsForTextIndicatorOptions(data.options);
 
-    data.contentImage = takeSnapshot(frame, snapshotRect, snapshotOptions, data.contentImageScaleFactor);
+    data.contentImage = takeSnapshot(frame, snapshotRect, snapshotOptions, data.contentImageScaleFactor, clipRectsInDocumentCoordinates);
     if (!data.contentImage)
         return false;
 
     if (data.options & TextIndicatorOptionIncludeSnapshotWithSelectionHighlight) {
         float snapshotScaleFactor;
-        data.contentImageWithHighlight = takeSnapshot(frame, snapshotRect, SnapshotOptionsNone, snapshotScaleFactor);
+        data.contentImageWithHighlight = takeSnapshot(frame, snapshotRect, SnapshotOptionsNone, snapshotScaleFactor, clipRectsInDocumentCoordinates);
         ASSERT(!data.contentImageWithHighlight || data.contentImageScaleFactor == snapshotScaleFactor);
     }
     
@@ -238,7 +238,7 @@ static bool initializeIndicator(TextIndicatorData& data, Frame& frame, const Ran
     data.textBoundingRectInRootViewCoordinates = textBoundingRectInRootViewCoordinates;
     data.textRectsInBoundingRectCoordinates = textRectsInBoundingRectCoordinates;
 
-    return takeSnapshots(data, frame, enclosingIntRect(textBoundingRectInDocumentCoordinates));
+    return takeSnapshots(data, frame, enclosingIntRect(textBoundingRectInDocumentCoordinates), textRects);
 }
 
 } // namespace WebCore
index f805c4c..cbaf29a 100644 (file)
@@ -1,3 +1,17 @@
+2016-03-22  Tim Horton  <timothy_horton@apple.com>
+
+        Invoking a link preview on a complex link (e.g. an image) results in an empty TextIndicator
+        https://bugs.webkit.org/show_bug.cgi?id=155779
+        <rdar://problem/22408793>
+
+        Reviewed by Simon Fraser.
+
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::dictionaryPopupInfoForRange):
+        (WebKit::WebPage::performImmediateActionHitTestAtLocation):
+        Use the TextIndicator mode where we give up on selection-only snapshotting
+        and just paint all content on Mac, similar to what we do for 3D Touch indicators.
+
 2016-03-22  Alex Christensen  <achristensen@webkit.org>
 
         Fix HTTPS on Mac using NSURLSession after r198457
index bf1751b..f4b2336 100644 (file)
@@ -443,7 +443,7 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& ra
         [scaledNSAttributedString addAttributes:scaledAttributes.get() range:range];
     }];
 
-    TextIndicatorOptions indicatorOptions = TextIndicatorOptionDefault;
+    TextIndicatorOptions indicatorOptions = TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges;
     if (presentationTransition == TextIndicatorPresentationTransition::BounceAndCrossfade)
         indicatorOptions |= TextIndicatorOptionIncludeSnapshotWithSelectionHighlight;
 
@@ -969,7 +969,7 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
     Element *URLElement = hitTestResult.URLElement();
     if (!absoluteLinkURL.isEmpty() && URLElement) {
         RefPtr<Range> linkRange = rangeOfContents(*URLElement);
-        immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorOptionDefault, TextIndicatorPresentationTransition::FadeIn);
+        immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges, TextIndicatorPresentationTransition::FadeIn);
     }
 
     NSDictionary *options = nil;
@@ -1005,7 +1005,7 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
             detectedDataBoundingBox.unite(frameView->contentsToWindow(quad.enclosingBoundingBox()));
 
         immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
-        immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, TextIndicatorOptionDefault, TextIndicatorPresentationTransition::FadeIn);
+        immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges, TextIndicatorPresentationTransition::FadeIn);
         immediateActionResult.detectedDataOriginatingPageOverlay = overlay->pageOverlayID();
 
         break;
@@ -1018,7 +1018,7 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
         immediateActionResult.detectedDataActionContext = DataDetection::detectItemAroundHitTestResult(hitTestResult, detectedDataBoundingBox, detectedDataRange);
         if (immediateActionResult.detectedDataActionContext && detectedDataRange) {
             immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
-            immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, TextIndicatorOptionDefault, TextIndicatorPresentationTransition::FadeIn);
+            immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges, TextIndicatorPresentationTransition::FadeIn);
         }
     }