Bug 15078

Summary: REGRESSION (r21387): Find on page can scroll overflow:hidden boxes to reveal found text
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: 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 Flags
Test case
none
More test cases
none
First attempt
none
Cross-port layout test
none
Patch
none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
Patch mitz: review-

Description mitz 2007-08-25 23:55:28 PDT
To reproduce this bug, using Safari 3 and TOT WebKit go to <http://www.apple.com/>, hit Command-F and type "download". The normally-invisible "download" link will become visible and remain visible thereafter.

The link is initially clipped out by an overflow:hidden box. Doing the find scrolls it into view. I think the regression happened in <http://trac.webkit.org/projects/webkit/changeset/21387>.
Comment 1 mitz 2007-08-25 23:59:10 PDT
Created attachment 16120 [details]
Test case
Comment 2 Mark Rowe (bdash) 2007-08-26 00:08:15 PDT
<rdar://problem/5438458>
Comment 3 Adele Peterson 2007-08-26 10:10:29 PDT
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?
Comment 4 mitz 2007-08-26 12:46:49 PDT
(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).
Comment 5 mitz 2007-08-26 13:20:42 PDT
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.
Comment 6 Rob Buis 2007-09-25 13:25:19 PDT
Created attachment 16390 [details]
First attempt

This should fix it.
Cheers,

Rob.
Comment 7 John Sullivan 2007-09-25 13:31:03 PDT
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 8 mitz 2007-09-25 13:47:26 PDT
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.
Comment 9 Benjamin Poulain 2010-10-21 06:26:50 PDT
Created attachment 71431 [details]
Cross-port layout test
Comment 10 Robert Hogan 2013-07-28 09:52:02 PDT
Created attachment 207609 [details]
Patch
Comment 11 Build Bot 2013-07-28 14:34:58 PDT
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
Comment 12 Build Bot 2013-07-28 14:35:00 PDT
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 13 Build Bot 2013-07-28 15:03:44 PDT
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
Comment 14 Build Bot 2013-07-28 15:03:47 PDT
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
Comment 15 Robert Hogan 2013-07-29 10:43:23 PDT
Created attachment 207663 [details]
Patch
Comment 16 mitz 2013-07-29 10:52:52 PDT
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.
Comment 17 Horia Dragmir 2014-10-20 16:16:30 PDT
If you're looking for another repro case, I made this jsbin demo: http://jsbin.com/wenajaxawe/1/
Comment 18 Ahmad Saleem 2023-02-16 13:29:06 PST
Using "Cross-port layout test", I get "failed" for Safari 16.3 while Chrome Canary 112 and Firefox Nightly 112 show "Passed". Thanks!