Bug 49888 – Focused <area> should use CSS properties of <area> instead of associated <img>

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Daniel Bates]

Currently, when focusing an <area> (say by pressing option + tab in Mac Safari) we use the CSS style information for the <img> associated with the <area>. Instead, we should use the CSS style information for the focused <area>. This will also make the styling behavior of a focused <area> consistent with that of a focused hyperlink. In particular, this will make the outline-color of a focused <area> effect the color of its focus ring.

Additional remarks:
By default, an HTML Image element is not focusable and an HTML Area element is focusable. As a result of the current behavior the default focus ring color (as specified by CSS pseudo-class :focus in the file WebCore/css/html.css) is not honored for HTML Area elements since we use the style information of the associated <img> when drawing the focus ring for the focused <area>.

[Daniel Bates]

Created attachment 74516 [details]
Patch with test cases

[Daniel Bates]

> Additional remarks:
> By default, an HTML Image element is not focusable and an HTML Area element is focusable. As a result of the current behavior the default focus ring color (as specified by CSS pseudo-class :focus in the file WebCore/css/html.css) is not honored for HTML Area elements since we use the style information of the associated <img> when drawing the focus ring for the focused <area>.

I should add that with the current behavior the default focus ring color for a focused <area> is black. Instead, it should be -webkit-focus-ring-color as it is for focused hyperlinks and form controls.

[Dave Hyatt]

Comment on attachment 74516 [details]
Patch with test cases

Let's not just make a throwaway render style here.  Fix the area element to do the right thing so we can get the style cached.

That means implementing:

virtual void setRenderStyle(PassRefPtr<RenderStyle>);

and


virtual RenderStyle* nonRendererRenderStyle() const;

Study HTMLOptionElement for an example.  This way you want repeatedly request the style every time you paint, since it will get properly cached.

[Daniel Bates]

Created attachment 74517 [details]
Patch with test cases

Updated patch based on Dave Hyatt's suggestions.

[Eric Seidel (no email)]

Attachment 74517 [details] did not build on mac:
Build output: http://queues.webkit.org/results/6221121

[Eric Seidel (no email)]

Attachment 74517 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6134105

[Daniel Bates]

Created attachment 74519 [details]
Patch with test cases

Include "NodeRenderStyle.h" in file RenderImage.cpp to fix the Mac Release build.

[Dave Hyatt]

Comment on attachment 74519 [details]
Patch with test cases

Actually I thought about this some more, and since Area is a child of Map, you'd have to patch HTMLMapElement also (if the area style depended on proper inheritance from the map style).

I think the other caching mechanism, Element::computedStyle(), will work better and result in a smaller patch, since it will properly inherit form HTMLMapElement's computedStyle().

Sorry to make you change things again, but I think removing all the renderStyle() stuff and just calling computedStyle() instead of styleForElement will work better.

[Daniel Bates]

(In reply to comment #8)
> (From update of attachment 74519 [details])
> Actually I thought about this some more, and since Area is a child of Map, you'd have to patch HTMLMapElement also (if the area style depended on proper inheritance from the map style).
> 

As per section 18.4 "Dynamic outlines: the 'outline' property" of the CSS2.1 spec. <http://www.w3.org/TR/CSS2/ui.html#dynamic-outlines>, the outline properties are not inherited.

[Daniel Bates]

Created attachment 75188 [details]
Patch with test cases

Updated patch to use computed style.

Added additional layout test, fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map.html, to ensure we don't inherit the outline-color property as per section 18.4 of the CSS 2.1 spec.

[Daniel Bates]

Created attachment 75197 [details]
Patch with test cases

Added test case fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map.html to test that explicitly inheriting the outline-color works.

[Dave Hyatt]

Comment on attachment 75197 [details]
Patch with test cases

r=me

[Daniel Bates]

Committed r72962: <http://trac.webkit.org/changeset/72962>