Bug 26364 - Search can find text that's hidden by overflow:hidden
: Search can find text that's hidden by overflow:hidden
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Text
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To: Darin Adler
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-12 17:58 PDT by Darin Adler
Modified: 2009-06-19 12:46 PDT (History)
2 users (show)

See Also:


Attachments
patch (31.09 KB, patch)
2009-06-13 07:55 PDT, Darin Adler
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2009-06-12 17:58:34 PDT
The site may be gone by the time you see this, but at http://developer.apple.com/wwdc/sessions/ if you search for "AppKit" you'll find something invisible. It's part of a hidden description.
Comment 1 Darin Adler 2009-06-12 17:59:53 PDT
<rdar://problem/6952081>
Comment 2 Darin Adler 2009-06-13 07:55:01 PDT
Created attachment 31236 [details]
patch
Comment 3 mitz@webkit.org 2009-06-13 10:23:11 PDT
Comment on attachment 31236 [details]
patch

> +static inline bool fullyClipsContents(Node* node)
> +{
> +    RenderObject* renderer = node->renderer();
> +    if (!renderer || !renderer->isBox())
> +        return false;
> +    RenderStyle* style = renderer->style();
> +    if (style->overflowX() == OVISIBLE || style->overflowY() == OVISIBLE)
> +        return false;
> +    return toRenderBox(renderer)->size().isEmpty();
> +}

I know that the change log says "We can add other cases here later", but why not handle cases like "overflow-x: hidden; width: 0;" and "overflow-y: hidden; height: 0;" now?

> +    // Push true if this node full clips its contents, or if a parent already has fully

Typo: "full clips".
Comment 4 Darin Adler 2009-06-14 11:18:36 PDT
(In reply to comment #3)
> I know that the change log says "We can add other cases here later", but why
> not handle cases like "overflow-x: hidden; width: 0;" and "overflow-y: hidden;
> height: 0;" now?

I tested those cases in the snippet editor, and in both cases no text was visible. That is also the rule the text iterator implements.
Comment 5 mitz@webkit.org 2009-06-14 11:22:41 PDT
Comment on attachment 31236 [details]
patch

(In reply to comment #4)
> (In reply to comment #3)
> > I know that the change log says "We can add other cases here later", but why
> > not handle cases like "overflow-x: hidden; width: 0;" and "overflow-y: hidden;
> > height: 0;" now?
> 
> I tested those cases in the snippet editor, and in both cases no text was
> visible. That is also the rule the text iterator implements.

Oh, I see: setting overflow-x to hidden sets overflow-y to something other than visible, so
+    if (style->overflowX() == OVISIBLE || style->overflowY() == OVISIBLE)
covers this case.
Comment 6 Darin Adler 2009-06-14 15:08:15 PDT
(In reply to comment #5)
> Oh, I see: setting overflow-x to hidden sets overflow-y to something other than
> visible, so
> +    if (style->overflowX() == OVISIBLE || style->overflowY() == OVISIBLE)
> covers this case.

I guess that || should really be an && -- I'll test to make sure that works and land it that way.
Comment 7 Darin Adler 2009-06-14 20:10:21 PDT
http://trac.webkit.org/changeset/44674
Comment 8 Finnur Thorarinsson 2009-06-17 17:16:59 PDT
Darin, I think we may have a problem with this patch...

See: http://code.google.com/p/chromium/issues/detail?id=14491
Comment 9 Darin Adler 2009-06-17 17:56:53 PDT
(In reply to comment #8)
> Darin, I think we may have a problem with this patch.
> 
> See: http://code.google.com/p/chromium/issues/detail?id=14491

I'd be happy to help fix it. If you can come up with a test case, feel free to put the bug in bugs.webkit.org and assign it to hem.
Comment 10 Finnur Thorarinsson 2009-06-19 12:46:32 PDT
Filed: https://bugs.webkit.org/show_bug.cgi?id=26557
If you need anything more, don't hesitate to contact me.