Bug 15078 - REGRESSION (r21387): Find on page can scroll overflow:hidden boxes to reveal found text
Summary: REGRESSION (r21387): Find on page can scroll overflow:hidden boxes to reveal ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Robert Hogan
URL: http://www.apple.com/
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-08-25 23:55 PDT by mitz
Modified: 2016-04-11 00:41 PDT (History)
12 users (show)

See Also:


Attachments
Test case (177 bytes, text/html)
2007-08-25 23:59 PDT, mitz
no flags Details
More test cases (711 bytes, text/html)
2007-08-26 13:20 PDT, mitz
no flags Details
First attempt (5.85 KB, patch)
2007-09-25 13:25 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Cross-port layout test (884 bytes, text/html)
2010-10-21 06:26 PDT, Benjamin Poulain
no flags Details
Patch (6.31 KB, patch)
2013-07-28 09:52 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (488.20 KB, application/zip)
2013-07-28 14:35 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (536.19 KB, application/zip)
2013-07-28 15:03 PDT, Build Bot
no flags Details
Patch (6.06 KB, patch)
2013-07-29 10:43 PDT, Robert Hogan
mitz: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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/