Bug 158395 – Find on page finds too many matches

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antti Koivisto]

WebKit may find non-visible text matches when doing find on page from browser UI. 

For example searching patch review view in bugs.webkit.org returns twice as many matches as there actually are on the page. This happens because the text content is replicated in an invisible subframe.

[Antti Koivisto]

rdar://problem/7440637

[Antti Koivisto]

Created attachment 280547 [details]
patch

[Brady Eidson]

👍👍👍

[Build Bot]

Comment on attachment 280547 [details]
patch

Attachment 280547 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1447562

New failing tests:
imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width.html

[Build Bot]

Created attachment 280551 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 280547 [details]
patch

Attachment 280547 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1447563

New failing tests:
imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width.html

[Build Bot]

Created attachment 280552 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4

[Darin Adler]

Comment on attachment 280547 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=280547&action=review

Wow, I am so excited about this one.

> Source/WebCore/editing/TextIterator.cpp:215
> +    return downcast<RenderBox>(*renderer).contentBoxRect().isEmpty();

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.

> Source/WebCore/editing/TextIterator.cpp:254
> +    auto* owner = document.ownerElement();
> +    while (owner) {

I’d like this better as a for loop:

    for (auto* owner = document.ownerElement(); owner; owner = owner->document().ownerElement()) {

> Source/WebCore/editing/TextIterator.cpp:1206
> +    , m_positionNode(nullptr)

I don’t think this is needed. The class definition already sets m_positionNode to nullptr.

> Source/WebCore/editing/TextIteratorBehavior.h:58
> +    // Makes visiblility test take into account the visibility of the frame.

Typo in the first use of the word "visibility".

[Build Bot]

Comment on attachment 280547 [details]
patch

Attachment 280547 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1447823

New failing tests:
imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width.html

[Build Bot]

Created attachment 280554 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 280547 [details]
patch

Attachment 280547 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1447912

New failing tests:
imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width.html

[Build Bot]

Created attachment 280555 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

Created attachment 280557 [details]
patch

[Antti Koivisto]

Created attachment 280558 [details]
patch

[Antti Koivisto]

https://trac.webkit.org/r201701