[Webkit-unassigned] [Bug 71788] <area>-tag within <map> can get focus when it is hidden

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=71788


Andy Estes <aestes at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #133194|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #16 from Andy Estes <aestes at apple.com>  2012-03-22 00:42:41 PST ---
(From update of attachment 133194)
View in context: https://bugs.webkit.org/attachment.cgi?id=133194&action=review

This looks good! r=me. I have a few comments that I'd like to see addressed before you land.

> Source/WebCore/ChangeLog:3
> +        <area>-tag within <map> can get focus when it is hidden.

I think we could should change the title of this bug to better capture the issue, since it's the <img> that's hidden, not the <map> or <area>. How about:

"An <area> element remains focusable even though its associated <img> is not rendered."

> Source/WebCore/ChangeLog:8
> +        The "HTMLAreaElement::isFocusable" method need to consider

This should read "HTMLAreaElement::isFocusable() needs to consider..."

> Source/WebCore/ChangeLog:16
> +        * html/HTMLAreaElement.h: Made imageElement() as const.

This should read "Make imageElement() const."

> Source/WebCore/html/HTMLAreaElement.cpp:204
> +    if (!(image && image->renderer()) || image->renderer()->style()->visibility() != VISIBLE)

I think this would read better as:

if (!image || image->renderer() || image->renderer()->style()->visibility() != VISIBLE)

> LayoutTests/ChangeLog:3
> +        <area>-tag within <map> can get focus when it is hidden.

Same comment about the bug title.

> LayoutTests/ChangeLog:8
> +        Tests to test the tab navigation.

"Tests to test..." doesn't sound right. You can remove this line, or say something like "Test sequential focus navigation."

> LayoutTests/fast/events/tab-test-not-visible-imagemap.html:49
> +    description("Testcase to test that tabbing does not focus area element which is not visible.");

You can remove "Testcase to test...". It reads fine as "Test that tabbing...".

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.