Bug 26364 – Search can find text that's hidden by overflow:hidden

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Darin Adler]

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.

[Darin Adler]

<rdar://problem/6952081>

[Darin Adler]

Created attachment 31236 [details]
patch

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".

[Darin Adler]

(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 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.

[Darin Adler]

(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.

[Darin Adler]

http://trac.webkit.org/changeset/44674

[Finnur Thorarinsson]

Darin, I think we may have a problem with this patch...

See: http://code.google.com/p/chromium/issues/detail?id=14491

[Darin Adler]

(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.

[Finnur Thorarinsson]

Filed: https://bugs.webkit.org/show_bug.cgi?id=26557
If you need anything more, don't hesitate to contact me.