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
: WebKit
Text
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To:
:
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2009-06-12 17:58 PST by
Modified: 2009-06-19 12:46 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-06-12 17:58:34 PST
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 From 2009-06-12 17:59:53 PST -------
<rdar://problem/6952081>
------- Comment #2 From 2009-06-13 07:55:01 PST -------
Created an attachment (id=31236) [details]
patch
------- Comment #3 From 2009-06-13 10:23:11 PST -------
(From update of attachment 31236 [details])
> +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 From 2009-06-14 11:18:36 PST -------
(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 From 2009-06-14 11:22:41 PST -------
(From update of attachment 31236 [details])
(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 From 2009-06-14 15:08:15 PST -------
(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 From 2009-06-14 20:10:21 PST -------
http://trac.webkit.org/changeset/44674
------- Comment #8 From 2009-06-17 17:16:59 PST -------
Darin, I think we may have a problem with this patch...

See: http://code.google.com/p/chromium/issues/detail?id=14491
------- Comment #9 From 2009-06-17 17:56:53 PST -------
(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 From 2009-06-19 12:46:32 PST -------
Filed: https://bugs.webkit.org/show_bug.cgi?id=26557
If you need anything more, don't hesitate to contact me.