Diff
Modified: branches/safari-604-branch/Source/WebCore/ChangeLog (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/ChangeLog 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/ChangeLog 2017-07-26 09:51:57 UTC (rev 219926)
@@ -1,5 +1,70 @@
2017-07-26 Jason Marcell <[email protected]>
+ Cherry-pick r219756. rdar://problem/33523829
+
+ 2017-07-21 Timothy Horton <[email protected]>
+
+ Drag and Drop preview image for Twitter link is the wrong shape
+ https://bugs.webkit.org/show_bug.cgi?id=174731
+ <rdar://problem/33335616>
+
+ Reviewed by Zalan Bujtas.
+
+ TextIndicator uses Range::borderAndTextQuads and ::absoluteTextRects
+ in order to get the rects of the indicated text. Currently, these
+ functions do not respect clipping, so clipped-out text (e.g. as seen
+ inside links on Twitter) generates lots of meaningless indicated rects.
+
+ * page/TextIndicator.cpp:
+ (WebCore::estimatedBackgroundColorForRange):
+ (WebCore::hasAnyIllegibleColors):
+ Change adjustTextIndicatorDataOptionsForEstimatedColorsIfNecessary
+ to instead be hasAnyIllegibleColors, and referred to in the same place
+ as hasNonInlineOrReplacedElements, so that it follows the same
+ upgrade path (leaving textRects empty, so that it is later filled in
+ with the absoluteBoundingRect). This was a mistake in r219033, which
+ instead would end up painting all content, but filling in textRects
+ with the actual individual text rects.
+
+ This alone changes the behavior on Twitter from lots of jagged misplaced
+ rects to a too-large bounding rect. Combined with the following changes,
+ the bounding rect is reduced to the right size:
+
+ (WebCore::initializeIndicator):
+ Adopt the new Range::borderAndTextQuads and ::absoluteTextRects parameter
+ and opt-in to respecting clipping for text rects.
+
+ * dom/DOMRectList.cpp:
+ (WebCore::DOMRectList::DOMRectList):
+ * dom/DOMRectList.h:
+ (WebCore::DOMRectList::create):
+ Add a DOMRectList constructor and create() that take FloatRects, similar
+ to the one that takes FloatQuads, but without the boundingRect() calls.
+
+ * dom/Document.h:
+ * dom/Document.cpp:
+ (WebCore::Document::convertAbsoluteToClientRects):
+ Add convertAbsoluteToClientRects, similar to covertAbsoluteToClientQuads,
+ except acting on rects instead of quads.
+
+ * dom/Range.cpp:
+ (WebCore::Range::absoluteRectsForRangeInText):
+ (WebCore::Range::absoluteTextRects):
+ (WebCore::Range::getClientRects):
+ (WebCore::Range::borderAndTextRects):
+ (WebCore::Range::boundingRect):
+ (WebCore::Range::absoluteBoundingRect):
+ (WebCore::Range::borderAndTextQuads): Deleted.
+ * dom/Range.h:
+ Replace borderAndTextQuads with borderAndTextRects, because all callers
+ just ended up calling boundingBox() on the quads.
+
+ Factor absoluteRectsForRangeInText out of absoluteTextRects and
+ borderAndTextQuads, and teach it to optionally intersect the text rects
+ with their renderer's absoluteClippedOverflowRect.
+
+2017-07-26 Jason Marcell <[email protected]>
+
Cherry-pick r219755. rdar://problem/33523843
2017-07-21 Per Arne Vollan <[email protected]>
Modified: branches/safari-604-branch/Source/WebCore/dom/DOMRectList.cpp (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/DOMRectList.cpp 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/DOMRectList.cpp 2017-07-26 09:51:57 UTC (rev 219926)
@@ -37,6 +37,13 @@
m_items.uncheckedAppend(DOMRect::create(quad.boundingBox()));
}
+DOMRectList::DOMRectList(const Vector<FloatRect>& rects)
+{
+ m_items.reserveInitialCapacity(rects.size());
+ for (auto& rect : rects)
+ m_items.uncheckedAppend(DOMRect::create(rect));
+}
+
DOMRectList::~DOMRectList()
{
}
Modified: branches/safari-604-branch/Source/WebCore/dom/DOMRectList.h (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/DOMRectList.h 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/DOMRectList.h 2017-07-26 09:51:57 UTC (rev 219926)
@@ -36,6 +36,7 @@
class DOMRectList : public RefCounted<DOMRectList> {
public:
static Ref<DOMRectList> create(const Vector<FloatQuad>& quads) { return adoptRef(*new DOMRectList(quads)); }
+ static Ref<DOMRectList> create(const Vector<FloatRect>& rects) { return adoptRef(*new DOMRectList(rects)); }
static Ref<DOMRectList> create() { return adoptRef(*new DOMRectList()); }
WEBCORE_EXPORT ~DOMRectList();
@@ -44,6 +45,7 @@
private:
WEBCORE_EXPORT explicit DOMRectList(const Vector<FloatQuad>& quads);
+ WEBCORE_EXPORT explicit DOMRectList(const Vector<FloatRect>& rects);
DOMRectList() = default;
Vector<Ref<DOMRect>> m_items;
Modified: branches/safari-604-branch/Source/WebCore/dom/Document.cpp (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/Document.cpp 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/Document.cpp 2017-07-26 09:51:57 UTC (rev 219926)
@@ -6559,6 +6559,23 @@
quad.move(documentToClientOffset);
}
}
+
+void Document::convertAbsoluteToClientRects(Vector<FloatRect>& rects, const RenderStyle& style)
+{
+ if (!view())
+ return;
+
+ auto& frameView = *view();
+ float inverseFrameScale = frameView.absoluteToDocumentScaleFactor(style.effectiveZoom());
+ auto documentToClientOffset = frameView.documentToClientOffset();
+
+ for (auto& rect : rects) {
+ if (inverseFrameScale != 1)
+ rect.scale(inverseFrameScale);
+
+ rect.move(documentToClientOffset);
+ }
+}
void Document::convertAbsoluteToClientRect(FloatRect& rect, const RenderStyle& style)
{
Modified: branches/safari-604-branch/Source/WebCore/dom/Document.h (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/Document.h 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/Document.h 2017-07-26 09:51:57 UTC (rev 219926)
@@ -1224,6 +1224,7 @@
#endif
void convertAbsoluteToClientQuads(Vector<FloatQuad>&, const RenderStyle&);
+ void convertAbsoluteToClientRects(Vector<FloatRect>&, const RenderStyle&);
void convertAbsoluteToClientRect(FloatRect&, const RenderStyle&);
bool hasActiveParser();
Modified: branches/safari-604-branch/Source/WebCore/dom/Range.cpp (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/Range.cpp 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/Range.cpp 2017-07-26 09:51:57 UTC (rev 219926)
@@ -1166,8 +1166,38 @@
return result;
}
-void Range::absoluteTextRects(Vector<IntRect>& rects, bool useSelectionHeight, RangeInFixedPosition* inFixed) const
+Vector<FloatRect> Range::absoluteRectsForRangeInText(Node* node, RenderText& renderText, bool useSelectionHeight, bool& isFixed, RespectClippingForTextRects respectClippingForTextRects) const
{
+ unsigned startOffset = node == &startContainer() ? m_start.offset() : 0;
+ unsigned endOffset = node == &endContainer() ? m_end.offset() : std::numeric_limits<unsigned>::max();
+
+ auto textQuads = renderText.absoluteQuadsForRange(startOffset, endOffset, useSelectionHeight, &isFixed);
+
+ if (respectClippingForTextRects == RespectClippingForTextRects::Yes) {
+ Vector<FloatRect> clippedRects;
+
+ auto absoluteClippedOverflowRect = renderText.absoluteClippedOverflowRect();
+
+ for (auto& quad : textQuads) {
+ auto clippedRect = intersection(quad.boundingBox(), absoluteClippedOverflowRect);
+ if (!clippedRect.isEmpty())
+ clippedRects.append(clippedRect);
+ }
+
+ return clippedRects;
+ }
+
+ Vector<FloatRect> floatRects;
+ floatRects.reserveInitialCapacity(textQuads.size());
+ for (auto& quad : textQuads)
+ floatRects.append(quad.boundingBox());
+ return floatRects;
+}
+
+void Range::absoluteTextRects(Vector<IntRect>& rects, bool useSelectionHeight, RangeInFixedPosition* inFixed, RespectClippingForTextRects respectClippingForTextRects) const
+{
+ // FIXME: This function should probably return FloatRects.
+
bool allFixed = true;
bool someFixed = false;
@@ -1180,15 +1210,15 @@
if (renderer->isBR())
renderer->absoluteRects(rects, flooredLayoutPoint(renderer->localToAbsolute()));
else if (is<RenderText>(*renderer)) {
- unsigned startOffset = node == &startContainer() ? m_start.offset() : 0;
- unsigned endOffset = node == &endContainer() ? m_end.offset() : std::numeric_limits<unsigned>::max();
- rects.appendVector(downcast<RenderText>(*renderer).absoluteRectsForRange(startOffset, endOffset, useSelectionHeight, &isFixed));
+ auto rectsForRenderer = absoluteRectsForRangeInText(node, downcast<RenderText>(*renderer), useSelectionHeight, isFixed, respectClippingForTextRects);
+ for (auto& rect : rectsForRenderer)
+ rects.append(enclosingIntRect(rect));
} else
continue;
allFixed &= isFixed;
someFixed |= isFixed;
}
-
+
if (inFixed)
*inFixed = allFixed ? EntirelyFixedPosition : (someFixed ? PartiallyFixedPosition : NotFixedPosition);
}
@@ -1765,7 +1795,7 @@
Ref<DOMRectList> Range::getClientRects() const
{
- return DOMRectList::create(borderAndTextQuads(CoordinateSpace::Client));
+ return DOMRectList::create(borderAndTextRects(CoordinateSpace::Client));
}
Ref<DOMRect> Range::getBoundingClientRect() const
@@ -1773,9 +1803,9 @@
return DOMRect::create(boundingRect(CoordinateSpace::Client));
}
-Vector<FloatQuad> Range::borderAndTextQuads(CoordinateSpace space) const
+Vector<FloatRect> Range::borderAndTextRects(CoordinateSpace space, RespectClippingForTextRects respectClippingForTextRects) const
{
- Vector<FloatQuad> quads;
+ Vector<FloatRect> rects;
ownerDocument().updateLayoutIgnorePendingStylesheets();
@@ -1799,36 +1829,38 @@
renderer->absoluteQuads(elementQuads);
if (space == CoordinateSpace::Client)
node->document().convertAbsoluteToClientQuads(elementQuads, renderer->style());
- quads.appendVector(elementQuads);
+
+ for (auto& quad : elementQuads)
+ rects.append(quad.boundingBox());
}
} else if (is<Text>(*node)) {
if (auto* renderer = downcast<Text>(*node).renderer()) {
- unsigned startOffset = node == &startContainer() ? m_start.offset() : 0;
- unsigned endOffset = node == &endContainer() ? m_end.offset() : std::numeric_limits<unsigned>::max();
- auto textQuads = renderer->absoluteQuadsForRange(startOffset, endOffset);
+ bool isFixed;
+ auto clippedRects = absoluteRectsForRangeInText(node, *renderer, false, isFixed, respectClippingForTextRects);
if (space == CoordinateSpace::Client)
- node->document().convertAbsoluteToClientQuads(textQuads, renderer->style());
- quads.appendVector(textQuads);
+ node->document().convertAbsoluteToClientRects(clippedRects, renderer->style());
+
+ rects.appendVector(clippedRects);
}
}
}
- return quads;
+ return rects;
}
-FloatRect Range::boundingRect(CoordinateSpace space) const
+FloatRect Range::boundingRect(CoordinateSpace space, RespectClippingForTextRects respectClippingForTextRects) const
{
FloatRect result;
- for (auto& quad : borderAndTextQuads(space))
- result.unite(quad.boundingBox());
+ for (auto& rect : borderAndTextRects(space, respectClippingForTextRects))
+ result.unite(rect);
return result;
}
-FloatRect Range::absoluteBoundingRect() const
+FloatRect Range::absoluteBoundingRect(RespectClippingForTextRects respectClippingForTextRects) const
{
- return boundingRect(CoordinateSpace::Absolute);
+ return boundingRect(CoordinateSpace::Absolute, respectClippingForTextRects);
}
-
+
TextStream& operator<<(TextStream& ts, const RangeBoundaryPoint& r)
{
return ts << r.toPosition();
Modified: branches/safari-604-branch/Source/WebCore/dom/Range.h (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/dom/Range.h 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/dom/Range.h 2017-07-26 09:51:57 UTC (rev 219926)
@@ -41,6 +41,7 @@
class FloatQuad;
class Node;
class NodeWithIndex;
+class RenderText;
class SelectionRect;
class Text;
class VisiblePosition;
@@ -116,12 +117,13 @@
};
// Not transform-friendly
- WEBCORE_EXPORT void absoluteTextRects(Vector<IntRect>&, bool useSelectionHeight = false, RangeInFixedPosition* = nullptr) const;
+ enum class RespectClippingForTextRects { No, Yes };
+ WEBCORE_EXPORT void absoluteTextRects(Vector<IntRect>&, bool useSelectionHeight = false, RangeInFixedPosition* = nullptr, RespectClippingForTextRects = RespectClippingForTextRects::No) const;
WEBCORE_EXPORT IntRect absoluteBoundingBox() const;
// Transform-friendly
WEBCORE_EXPORT void absoluteTextQuads(Vector<FloatQuad>&, bool useSelectionHeight = false, RangeInFixedPosition* = nullptr) const;
- WEBCORE_EXPORT FloatRect absoluteBoundingRect() const;
+ WEBCORE_EXPORT FloatRect absoluteBoundingRect(RespectClippingForTextRects = RespectClippingForTextRects::No) const;
#if PLATFORM(IOS)
WEBCORE_EXPORT void collectSelectionRects(Vector<SelectionRect>&) const;
WEBCORE_EXPORT int collectSelectionRectsWithoutUnionInteriorLines(Vector<SelectionRect>&) const;
@@ -162,9 +164,11 @@
ExceptionOr<RefPtr<DocumentFragment>> processContents(ActionType);
enum class CoordinateSpace { Absolute, Client };
- Vector<FloatQuad> borderAndTextQuads(CoordinateSpace) const;
- FloatRect boundingRect(CoordinateSpace) const;
+ Vector<FloatRect> borderAndTextRects(CoordinateSpace, RespectClippingForTextRects = RespectClippingForTextRects::No) const;
+ FloatRect boundingRect(CoordinateSpace, RespectClippingForTextRects = RespectClippingForTextRects::No) const;
+ Vector<FloatRect> absoluteRectsForRangeInText(Node*, RenderText&, bool useSelectionHeight, bool& isFixed, RespectClippingForTextRects) const;
+
Ref<Document> m_ownerDocument;
RangeBoundaryPoint m_start;
RangeBoundaryPoint m_end;
Modified: branches/safari-604-branch/Source/WebCore/page/TextIndicator.cpp (219925 => 219926)
--- branches/safari-604-branch/Source/WebCore/page/TextIndicator.cpp 2017-07-26 09:51:53 UTC (rev 219925)
+++ branches/safari-604-branch/Source/WebCore/page/TextIndicator.cpp 2017-07-26 09:51:57 UTC (rev 219926)
@@ -237,7 +237,7 @@
commonAncestor = commonAncestor->parentOrShadowHostElement();
}
- auto boundingRectForRange = enclosingIntRect(range.absoluteBoundingRect());
+ auto boundingRectForRange = enclosingIntRect(range.absoluteBoundingRect(Range::RespectClippingForTextRects::Yes));
Vector<Color> parentRendererBackgroundColors;
for (; !!renderer; renderer = renderer->parent()) {
auto absoluteBoundingBox = renderer->absoluteBoundingBoxRect();
@@ -259,14 +259,17 @@
return estimatedBackgroundColor;
}
-static void adjustTextIndicatorDataOptionsForEstimatedColorsIfNecessary(TextIndicatorData& data, const Color& backgroundColor, HashSet<Color>&& textColors)
+static bool hasAnyIllegibleColors(TextIndicatorData& data, const Color& backgroundColor, HashSet<Color>&& textColors)
{
if (data.options & TextIndicatorOptionPaintAllContent)
- return;
+ return false;
if (!(data.options & TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges))
- return;
+ return false;
+ if (!(data.options & TextIndicatorOptionComputeEstimatedBackgroundColor))
+ return false;
+
bool hasOnlyLegibleTextColors = true;
if (data.options & TextIndicatorOptionRespectTextColor) {
for (auto& textColor : textColors) {
@@ -277,11 +280,7 @@
} else
hasOnlyLegibleTextColors = textColorIsLegibleAgainstBackgroundColor(Color::black, backgroundColor);
- if (!hasOnlyLegibleTextColors || !textColors.size()) {
- // If the text color is not legible against the estimated color, force all content to be painted.
- data.options &= ~TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges;
- data.options |= TextIndicatorOptionPaintAllContent;
- }
+ return !hasOnlyLegibleTextColors || textColors.isEmpty();
}
static bool initializeIndicator(TextIndicatorData& data, Frame& frame, const Range& range, FloatSize margin, bool indicatesCurrentSelection)
@@ -289,9 +288,10 @@
if (auto* document = frame.document())
document->updateLayoutIgnorePendingStylesheets();
+ bool treatRangeAsComplexDueToIllegibleTextColors = false;
if (data.options & TextIndicatorOptionComputeEstimatedBackgroundColor) {
data.estimatedBackgroundColor = estimatedBackgroundColorForRange(range, frame);
- adjustTextIndicatorDataOptionsForEstimatedColorsIfNecessary(data, data.estimatedBackgroundColor, estimatedTextColorsForRange(range));
+ treatRangeAsComplexDueToIllegibleTextColors = hasAnyIllegibleColors(data, data.estimatedBackgroundColor, estimatedTextColorsForRange(range));
}
Vector<FloatRect> textRects;
@@ -304,17 +304,22 @@
FrameSelection::TextRectangleHeight textRectHeight = (data.options & TextIndicatorOptionTightlyFitContent) ? FrameSelection::TextRectangleHeight::TextHeight : FrameSelection::TextRectangleHeight::SelectionHeight;
- if ((data.options & TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges) && hasNonInlineOrReplacedElements(range))
+ if ((data.options & TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges) && (hasNonInlineOrReplacedElements(range) || treatRangeAsComplexDueToIllegibleTextColors))
data.options |= TextIndicatorOptionPaintAllContent;
#if PLATFORM(IOS)
else if (data.options & TextIndicatorOptionUseSelectionRectForSizing)
getSelectionRectsForRange(textRects, range);
#endif
- else
- frame.selection().getTextRectangles(textRects, textRectHeight);
+ else {
+ Vector<IntRect> absoluteTextRects;
+ range.absoluteTextRects(absoluteTextRects, textRectHeight == FrameSelection::TextRectangleHeight::SelectionHeight, nullptr, Range::RespectClippingForTextRects::Yes);
+ for (auto& rect : absoluteTextRects)
+ textRects.append(rect);
+ }
+
if (textRects.isEmpty())
- textRects.append(range.absoluteBoundingRect());
+ textRects.append(range.absoluteBoundingRect(Range::RespectClippingForTextRects::Yes));
auto frameView = frame.view();
