Summary: | REGRESSION (r21387): Find on page can scroll overflow:hidden boxes to reveal found text | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Robert Hogan <robert> | ||||||||||||||||||
Status: | NEW --- | ||||||||||||||||||||
Severity: | Normal | CC: | adele, ahmad.saleem792, alluse, bdakin, benjamin, buildbot, horia, johnme, karlcow, rniwa, robert, simon.fraser, sullivan, tonikitoo, zalan | ||||||||||||||||||
Priority: | P1 | Keywords: | HasReduction, InRadar, Regression | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||
OS: | All | ||||||||||||||||||||
URL: | http://www.apple.com/ | ||||||||||||||||||||
Attachments: |
|
Description
mitz
2007-08-25 23:55:28 PDT
Created attachment 16120 [details]
Test case
I think we may have done this intentionally...I'm not sure. Or maybe we only meant to scroll to reveal overflow:hidden if its editable? What do other browsers do? (In reply to comment #3) > What do other browsers do? Firefox doesn't scroll. WinIE doesn't scroll the apple.com page but does scroll the test case (attachment 16120 [details]), so the test case is not quite representative of the page. I noticed that in WinIE, on the Apple site I can't change the scrollTop property of the overflow (it just fails silently). Created attachment 16127 [details]
More test cases
The middle one is the one that resembles apple.com. I think WinIE's behavior is too crazy to try to match.
Created attachment 16390 [details]
First attempt
This should fix it.
Cheers,
Rob.
It does seem extremely likely that this issue was caused by <http://trac.webkit.org/projects/webkit/changeset/21387>. Before checking in any change here, it's critical to make sure that the specific bug fixed by that change (searching for text on RSS pages) remains fixed. Comment on attachment 16390 [details]
First attempt
This does not seem right at all. The bug is about scrolling, or perhaps about clipped-out text. The patch just misses all text, clipped out or not, in overflow:hidden boxes.
Created attachment 71431 [details]
Cross-port layout test
Created attachment 207609 [details]
Patch
Comment on attachment 207609 [details] Patch Attachment 207609 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1248827 New failing tests: fast/overflow/find-ignores-invisible-text-in-overflow-hidden-containers.html fast/flexbox/line-clamp-link-after-ellipsis.html Created attachment 207612 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Comment on attachment 207609 [details] Patch Attachment 207609 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1255760 New failing tests: fast/overflow/find-ignores-invisible-text-in-overflow-hidden-containers.html fast/flexbox/line-clamp-link-after-ellipsis.html Created attachment 207613 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Created attachment 207663 [details]
Patch
Comment on attachment 207663 [details]
Patch
I am not sure what is the intended behavior with this patch, and I don’t know whether it’s desirable. This patch is not just “ignoring hits”, it is eliding text from everything that uses the text iterator (such as innerText and the plain-text pasteboard item). It also tries to consider the entire text node, of which some lines (or parts thereof) may be clipped out, and others may not, and the match may occur in either the former or the latter. It also considers overflow-y and not overflow-x, and looks at the renderer’s enclosing layer and ignores other ancestor layers.
If you're looking for another repro case, I made this jsbin demo: http://jsbin.com/wenajaxawe/1/ Using "Cross-port layout test", I get "failed" for Safari 16.3 while Chrome Canary 112 and Firefox Nightly 112 show "Passed". Thanks! |