<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[188527] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/188527">188527</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2015-08-17 11:12:08 -0700 (Mon, 17 Aug 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomDocumentMarkerControllercpp">trunk/Source/WebCore/dom/DocumentMarkerController.cpp</a></li>
<li><a href="#trunkSourceWebCoredomRenderedDocumentMarkerh">trunk/Source/WebCore/dom/RenderedDocumentMarker.h</a></li>
<li><a href="#trunkSourceWebCorerenderingInlineTextBoxcpp">trunk/Source/WebCore/rendering/InlineTextBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingsvgSVGInlineFlowBoxcpp">trunk/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextmarkmatchesbrokenlinerenderingexpectedhtml">trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasttextmarkmatchesbrokenlinerenderinghtml">trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-08-17 17:50:09 UTC (rev 188526)
+++ trunk/LayoutTests/ChangeLog 2015-08-17 18:12:08 UTC (rev 188527)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2015-08-17 Timothy Horton <timothy_horton@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2015-08-17 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Accessing HTMLCollection.length is slow
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextmarkmatchesbrokenlinerenderingexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering-expected.html (0 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,14 @@
</span><ins>+<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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextmarkmatchesbrokenlinerenderinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/mark-matches-broken-line-rendering.html (0 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+<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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,5 +1,48 @@
</span><span class="cx"> 2015-08-17 Timothy Horton <timothy_horton@apple.com>
</span><span class="cx">
</span><ins>+ 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 <timothy_horton@apple.com>
+
</ins><span class="cx"> Adopt shrink-wrapping for TextIndicators on Mac
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=148064
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentMarkerControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/DocumentMarkerController.cpp (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -102,7 +102,7 @@
</span><span class="cx"> // matches off-screen are (that haven't been painted yet).
</span><span class="cx"> Node* node = textPiece->startContainer();
</span><span class="cx"> Vector<RenderedDocumentMarker*> markers = markersFor(node);
</span><del>- markers[markers.size() - 1]->setRenderedRect(range->absoluteBoundingBox());
</del><ins>+ markers[markers.size() - 1]->addRenderedRect(range->absoluteBoundingBox());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="lines">@@ -439,10 +439,7 @@
</span><span class="cx"> if (marker.type() != markerType)
</span><span class="cx"> continue;
</span><span class="cx">
</span><del>- if (!marker.isRendered())
- continue;
-
- result.append(marker.renderedRect());
</del><ins>+ result.appendVector(marker.renderedRects());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoredomRenderedDocumentMarkerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/RenderedDocumentMarker.h (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -28,39 +28,43 @@
</span><span class="cx"> #define RenderedDocumentMarker_h
</span><span class="cx">
</span><span class="cx"> #include "DocumentMarker.h"
</span><ins>+#include <wtf/Vector.h>
</ins><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> class RenderedDocumentMarker : public DocumentMarker {
</span><span class="cx"> public:
</span><del>-
</del><span class="cx"> explicit RenderedDocumentMarker(const DocumentMarker& marker)
</span><del>- : DocumentMarker(marker), m_renderedRect(invalidMarkerRect())
</del><ins>+ : DocumentMarker(marker)
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>- 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(); }
</del><ins>+ bool contains(const LayoutPoint& point) const
+ {
+ for (const auto& rect : m_renderedRects) {
+ if (rect.contains(point))
+ return true;
+ }
+ return false;
+ }
</ins><span class="cx">
</span><del>-private:
- static const LayoutRect& invalidMarkerRect()
</del><ins>+ void addRenderedRect(const LayoutRect& r) { m_renderedRects.append(r); }
+ const Vector<LayoutRect, 1>& renderedRects() const { return m_renderedRects; }
+ void invalidate(const LayoutRect& r)
</ins><span class="cx"> {
</span><del>- static const LayoutRect rect = LayoutRect(-1, -1, -1, -1);
- return rect;
</del><ins>+ for (const auto& rect : m_renderedRects) {
+ if (rect.intersects(r)) {
+ invalidate();
+ return;
+ }
+ }
</ins><span class="cx"> }
</span><ins>+ void invalidate() { m_renderedRects.clear(); }
</ins><span class="cx">
</span><del>- LayoutRect m_renderedRect;
</del><ins>+private:
+ Vector<LayoutRect, 1> m_renderedRects;
</ins><span class="cx"> };
</span><span class="cx">
</span><del>-inline void RenderedDocumentMarker::invalidate(const LayoutRect& r)
-{
- if (m_renderedRect.intersects(r))
- invalidate();
-}
</del><span class="cx">
</span><span class="cx"> } // namespace
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingInlineTextBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1104,7 +1104,7 @@
</span><span class="cx"> if (grammar || isDictationMarker) {
</span><span class="cx"> markerRect.move(-boxOrigin.x(), -boxOrigin.y());
</span><span class="cx"> markerRect = renderer().localToAbsoluteQuad(FloatRect(markerRect)).enclosingBoundingBox();
</span><del>- marker.setRenderedRect(markerRect);
</del><ins>+ marker.addRenderedRect(markerRect);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1132,8 +1132,8 @@
</span><span class="cx"> {
</span><span class="cx"> LayoutUnit selectionHeight = this->selectionHeight();
</span><span class="cx">
</span><del>- int sPos = std::max(marker.startOffset() - m_start, (unsigned)0);
- int ePos = std::min(marker.endOffset() - m_start, (unsigned)m_len);
</del><ins>+ int sPos = std::max<int>(marker.startOffset() - m_start, 0);
+ int ePos = std::min<int>(marker.endOffset() - m_start, m_len);
</ins><span class="cx"> TextRun run = constructTextRun(style, font);
</span><span class="cx">
</span><span class="cx"> // Always compute and store the rect associated with this marker. The computed rect is in absolute coordinates.
</span><span class="lines">@@ -1142,7 +1142,7 @@
</span><span class="cx"> font.adjustSelectionRectForText(run, renderedRect, sPos, ePos);
</span><span class="cx"> IntRect markerRect = enclosingIntRect(renderedRect);
</span><span class="cx"> markerRect = renderer().localToAbsoluteQuad(FloatQuad(markerRect)).enclosingBoundingBox();
</span><del>- marker.setRenderedRect(markerRect);
</del><ins>+ marker.addRenderedRect(markerRect);
</ins><span class="cx">
</span><span class="cx"> // Optionally highlight the text
</span><span class="cx"> if (renderer().frame().editor().markedTextMatchesAreHighlighted()) {
</span><span class="lines">@@ -1174,7 +1174,7 @@
</span><span class="cx"> font.adjustSelectionRectForText(run, selectionRect, sPos, ePos);
</span><span class="cx"> IntRect markerRect = enclosingIntRect(selectionRect);
</span><span class="cx"> markerRect = renderer().localToAbsoluteQuad(FloatRect(markerRect)).enclosingBoundingBox();
</span><del>- marker.setRenderedRect(markerRect);
</del><ins>+ marker.addRenderedRect(markerRect);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, bool background)
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingsvgSVGInlineFlowBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp (188526 => 188527)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -124,7 +124,7 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>- marker->setRenderedRect(textRenderer->localToAbsoluteQuad(markerRect).enclosingBoundingBox());
</del><ins>+ marker->addRenderedRect(textRenderer->localToAbsoluteQuad(markerRect).enclosingBoundingBox());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>