[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





--- Comment #18 from Antaryami Pandia <antaryami.pandia at motorola.com>  2012-03-22 01:34:52 PST ---
(In reply to comment #16)
> 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."

Right. But I can't modify the title, since I am not the reporter.
Can you or the reporter please do the same.

> > Source/WebCore/ChangeLog:8
> > +        The "HTMLAreaElement::isFocusable" method need to consider
> 
> This should read "HTMLAreaElement::isFocusable() needs to consider..."

will do.

> > Source/WebCore/ChangeLog:16
> > +        * html/HTMLAreaElement.h: Made imageElement() as const.
> 
> This should read "Make imageElement() const."

will do.

> > 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)

will do as your suggested in last comment.
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.

will do.

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

ok.

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

Ok.

I will upload a new patch once the title is modified.

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