- Revision
- 188527
- Author
- [email protected]
- Date
- 2015-08-17 11:12:08 -0700 (Mon, 17 Aug 2015)
Log Message
Holes for find matches that span multiple lines are completely wrong
https://bugs.webkit.org/show_bug.cgi?id=148065
<rdar://problem/5305002>
Reviewed by Simon Fraser.
Test: fast/text/mark-matches-broken-line-rendering.html
Two big problems with find holes:
- RenderedDocumentMarker only has one rect, but a marker can be painted
by multiple text runs, each with their own rendered rect
- paintTextMatchMarker does std::max((unsigned)a, (unsigned)0), which is
obviously an overflow problem/not going to work if a is negative
The combination of these issues causes the holes for find matches to be
very broken: text that isn't part of the match is highlighted, and some
text that is part of the match isn't highlighted.
To fix, make RenderedDocumentMarker support multiple rects, and use signed
math (like paintDocumentMarker already did) when computing positions in paintTextMatchMarker.
* dom/DocumentMarkerController.cpp:
(WebCore::DocumentMarkerController::addTextMatchMarker):
(WebCore::DocumentMarkerController::renderedRectsForMarkers):
* dom/RenderedDocumentMarker.h:
(WebCore::RenderedDocumentMarker::RenderedDocumentMarker):
(WebCore::RenderedDocumentMarker::isRendered):
(WebCore::RenderedDocumentMarker::contains):
(WebCore::RenderedDocumentMarker::addRenderedRect):
(WebCore::RenderedDocumentMarker::renderedRects):
(WebCore::RenderedDocumentMarker::invalidate):
(WebCore::RenderedDocumentMarker::setRenderedRect): Deleted.
(WebCore::RenderedDocumentMarker::renderedRect): Deleted.
(WebCore::RenderedDocumentMarker::invalidMarkerRect): Deleted.
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paintDocumentMarker):
(WebCore::InlineTextBox::paintTextMatchMarker):
(WebCore::InlineTextBox::computeRectForReplacementMarker):
* rendering/svg/SVGInlineFlowBox.cpp:
(WebCore::SVGInlineFlowBox::computeTextMatchMarkerRectForRenderer):
* fast/text/mark-matches-broken-line-rendering-expected.html: Added.
* fast/text/mark-matches-broken-line-rendering.html: Added.
Add a test that ensures that we correctly mark test matches that cross
line breaks.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (188526 => 188527)
--- trunk/LayoutTests/ChangeLog 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/LayoutTests/ChangeLog 2015-08-17 18:12:08 UTC (rev 188527)
@@ -1,3 +1,16 @@
+2015-08-17 Timothy Horton <[email protected]>
+
+ Holes for find matches that span multiple lines are completely wrong
+ https://bugs.webkit.org/show_bug.cgi?id=148065
+ <rdar://problem/5305002>
+
+ Reviewed by Simon Fraser.
+
+ * fast/text/mark-matches-broken-line-rendering-expected.html: Added.
+ * fast/text/mark-matches-broken-line-rendering.html: Added.
+ Add a test that ensures that we correctly mark test matches that cross
+ line breaks.
+
2015-08-17 Chris Dumez <[email protected]>
Accessing HTMLCollection.length is slow
Added: trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering-expected.html (0 => 188527)
--- trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering-expected.html 2015-08-17 18:12:08 UTC (rev 188527)
@@ -0,0 +1,14 @@
+<style>
+p {
+ display: inline-block;
+ background-color: blue;
+ width: 60px;
+}
+
+span {
+ background-color: yellow;
+}
+</style>
+<p>
+<span>Quo</span> <span>usque</span> <span>tandem</span> <span>abutere,</span> <span>Catilina,</span> <span>patientia</span> <span>nostra?</span>
+</p>
Added: trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering.html (0 => 188527)
--- trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering.html (rev 0)
+++ trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering.html 2015-08-17 18:12:08 UTC (rev 188527)
@@ -0,0 +1,16 @@
+<style>
+p {
+ display: inline-block;
+ background-color: blue;
+ width: 60px;
+}
+</style>
+<p>
+Quo usque tandem abutere, Catilina, patientia nostra?
+</p>
+<script>
+if (window.internals) {
+ internals.setMarkedTextMatchesAreHighlighted(true);
+ internals.countMatchesForText("Quo usque tandem abutere, Catilina, patientia nostra?", 0, "mark");
+}
+</script>
Modified: trunk/Source/WebCore/ChangeLog (188526 => 188527)
--- trunk/Source/WebCore/ChangeLog 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/Source/WebCore/ChangeLog 2015-08-17 18:12:08 UTC (rev 188527)
@@ -1,5 +1,48 @@
2015-08-17 Timothy Horton <[email protected]>
+ Holes for find matches that span multiple lines are completely wrong
+ https://bugs.webkit.org/show_bug.cgi?id=148065
+ <rdar://problem/5305002>
+
+ Reviewed by Simon Fraser.
+
+ Test: fast/text/mark-matches-broken-line-rendering.html
+
+ Two big problems with find holes:
+ - RenderedDocumentMarker only has one rect, but a marker can be painted
+ by multiple text runs, each with their own rendered rect
+ - paintTextMatchMarker does std::max((unsigned)a, (unsigned)0), which is
+ obviously an overflow problem/not going to work if a is negative
+
+ The combination of these issues causes the holes for find matches to be
+ very broken: text that isn't part of the match is highlighted, and some
+ text that is part of the match isn't highlighted.
+
+ To fix, make RenderedDocumentMarker support multiple rects, and use signed
+ math (like paintDocumentMarker already did) when computing positions in paintTextMatchMarker.
+
+ * dom/DocumentMarkerController.cpp:
+ (WebCore::DocumentMarkerController::addTextMatchMarker):
+ (WebCore::DocumentMarkerController::renderedRectsForMarkers):
+ * dom/RenderedDocumentMarker.h:
+ (WebCore::RenderedDocumentMarker::RenderedDocumentMarker):
+ (WebCore::RenderedDocumentMarker::isRendered):
+ (WebCore::RenderedDocumentMarker::contains):
+ (WebCore::RenderedDocumentMarker::addRenderedRect):
+ (WebCore::RenderedDocumentMarker::renderedRects):
+ (WebCore::RenderedDocumentMarker::invalidate):
+ (WebCore::RenderedDocumentMarker::setRenderedRect): Deleted.
+ (WebCore::RenderedDocumentMarker::renderedRect): Deleted.
+ (WebCore::RenderedDocumentMarker::invalidMarkerRect): Deleted.
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::paintDocumentMarker):
+ (WebCore::InlineTextBox::paintTextMatchMarker):
+ (WebCore::InlineTextBox::computeRectForReplacementMarker):
+ * rendering/svg/SVGInlineFlowBox.cpp:
+ (WebCore::SVGInlineFlowBox::computeTextMatchMarkerRectForRenderer):
+
+2015-08-17 Timothy Horton <[email protected]>
+
Adopt shrink-wrapping for TextIndicators on Mac
https://bugs.webkit.org/show_bug.cgi?id=148064
Modified: trunk/Source/WebCore/dom/DocumentMarkerController.cpp (188526 => 188527)
--- trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2015-08-17 18:12:08 UTC (rev 188527)
@@ -102,7 +102,7 @@
// matches off-screen are (that haven't been painted yet).
Node* node = textPiece->startContainer();
Vector<RenderedDocumentMarker*> markers = markersFor(node);
- markers[markers.size() - 1]->setRenderedRect(range->absoluteBoundingBox());
+ markers[markers.size() - 1]->addRenderedRect(range->absoluteBoundingBox());
}
}
}
@@ -439,10 +439,7 @@
if (marker.type() != markerType)
continue;
- if (!marker.isRendered())
- continue;
-
- result.append(marker.renderedRect());
+ result.appendVector(marker.renderedRects());
}
}
Modified: trunk/Source/WebCore/dom/RenderedDocumentMarker.h (188526 => 188527)
--- trunk/Source/WebCore/dom/RenderedDocumentMarker.h 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/Source/WebCore/dom/RenderedDocumentMarker.h 2015-08-17 18:12:08 UTC (rev 188527)
@@ -28,39 +28,43 @@
#define RenderedDocumentMarker_h
#include "DocumentMarker.h"
+#include <wtf/Vector.h>
namespace WebCore {
class RenderedDocumentMarker : public DocumentMarker {
public:
-
explicit RenderedDocumentMarker(const DocumentMarker& marker)
- : DocumentMarker(marker), m_renderedRect(invalidMarkerRect())
+ : DocumentMarker(marker)
{
}
- bool isRendered() const { return invalidMarkerRect() != m_renderedRect; }
- bool contains(const LayoutPoint& point) const { return isRendered() && m_renderedRect.contains(point); }
- void setRenderedRect(const LayoutRect& r) { m_renderedRect = r; }
- const LayoutRect& renderedRect() const { return m_renderedRect; }
- void invalidate(const LayoutRect&);
- void invalidate() { m_renderedRect = invalidMarkerRect(); }
+ bool contains(const LayoutPoint& point) const
+ {
+ for (const auto& rect : m_renderedRects) {
+ if (rect.contains(point))
+ return true;
+ }
+ return false;
+ }
-private:
- static const LayoutRect& invalidMarkerRect()
+ void addRenderedRect(const LayoutRect& r) { m_renderedRects.append(r); }
+ const Vector<LayoutRect, 1>& renderedRects() const { return m_renderedRects; }
+ void invalidate(const LayoutRect& r)
{
- static const LayoutRect rect = LayoutRect(-1, -1, -1, -1);
- return rect;
+ for (const auto& rect : m_renderedRects) {
+ if (rect.intersects(r)) {
+ invalidate();
+ return;
+ }
+ }
}
+ void invalidate() { m_renderedRects.clear(); }
- LayoutRect m_renderedRect;
+private:
+ Vector<LayoutRect, 1> m_renderedRects;
};
-inline void RenderedDocumentMarker::invalidate(const LayoutRect& r)
-{
- if (m_renderedRect.intersects(r))
- invalidate();
-}
} // namespace
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (188526 => 188527)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2015-08-17 18:12:08 UTC (rev 188527)
@@ -1104,7 +1104,7 @@
if (grammar || isDictationMarker) {
markerRect.move(-boxOrigin.x(), -boxOrigin.y());
markerRect = renderer().localToAbsoluteQuad(FloatRect(markerRect)).enclosingBoundingBox();
- marker.setRenderedRect(markerRect);
+ marker.addRenderedRect(markerRect);
}
}
@@ -1132,8 +1132,8 @@
{
LayoutUnit selectionHeight = this->selectionHeight();
- int sPos = std::max(marker.startOffset() - m_start, (unsigned)0);
- int ePos = std::min(marker.endOffset() - m_start, (unsigned)m_len);
+ int sPos = std::max<int>(marker.startOffset() - m_start, 0);
+ int ePos = std::min<int>(marker.endOffset() - m_start, m_len);
TextRun run = constructTextRun(style, font);
// Always compute and store the rect associated with this marker. The computed rect is in absolute coordinates.
@@ -1142,7 +1142,7 @@
font.adjustSelectionRectForText(run, renderedRect, sPos, ePos);
IntRect markerRect = enclosingIntRect(renderedRect);
markerRect = renderer().localToAbsoluteQuad(FloatQuad(markerRect)).enclosingBoundingBox();
- marker.setRenderedRect(markerRect);
+ marker.addRenderedRect(markerRect);
// Optionally highlight the text
if (renderer().frame().editor().markedTextMatchesAreHighlighted()) {
@@ -1174,7 +1174,7 @@
font.adjustSelectionRectForText(run, selectionRect, sPos, ePos);
IntRect markerRect = enclosingIntRect(selectionRect);
markerRect = renderer().localToAbsoluteQuad(FloatRect(markerRect)).enclosingBoundingBox();
- marker.setRenderedRect(markerRect);
+ marker.addRenderedRect(markerRect);
}
void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, bool background)
Modified: trunk/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp (188526 => 188527)
--- trunk/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp 2015-08-17 18:12:08 UTC (rev 188527)
@@ -124,7 +124,7 @@
}
}
- marker->setRenderedRect(textRenderer->localToAbsoluteQuad(markerRect).enclosingBoundingBox());
+ marker->addRenderedRect(textRenderer->localToAbsoluteQuad(markerRect).enclosingBoundingBox());
}
}
