<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Find on page finds too many matches"
href="https://bugs.webkit.org/show_bug.cgi?id=158395#c8">Comment # 8</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Find on page finds too many matches"
href="https://bugs.webkit.org/show_bug.cgi?id=158395">bug 158395</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=280547&action=diff" name="attach_280547" title="patch">attachment 280547</a> <a href="attachment.cgi?id=280547&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=280547&action=review">https://bugs.webkit.org/attachment.cgi?id=280547&action=review</a>
Wow, I am so excited about this one.
<span class="quote">> Source/WebCore/editing/TextIterator.cpp:215
> + return downcast<RenderBox>(*renderer).contentBoxRect().isEmpty();</span >
It’s a shame that this has to compute the position of the content box, since the code doesn’t need it. The width and height are always just contentWidth() and contentHeight(). If there was a helper that just returned that we could have used it.
<span class="quote">> Source/WebCore/editing/TextIterator.cpp:254
> + auto* owner = document.ownerElement();
> + while (owner) {</span >
I’d like this better as a for loop:
for (auto* owner = document.ownerElement(); owner; owner = owner->document().ownerElement()) {
<span class="quote">> Source/WebCore/editing/TextIterator.cpp:1206
> + , m_positionNode(nullptr)</span >
I don’t think this is needed. The class definition already sets m_positionNode to nullptr.
<span class="quote">> Source/WebCore/editing/TextIteratorBehavior.h:58
> + // Makes visiblility test take into account the visibility of the frame.</span >
Typo in the first use of the word "visibility".</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>