[webkit-reviews] review granted: [Bug 200997] SVG element should become focusable when focus and key event listeners are added : [Attachment 376945] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 200997: SVG element should become focusable when focus and key event
listeners are added
https://bugs.webkit.org/show_bug.cgi?id=200997

Attachment 376945: Patch

https://bugs.webkit.org/attachment.cgi?id=376945&action=review




--- Comment #2 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 376945
  --> https://bugs.webkit.org/attachment.cgi?id=376945
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376945&action=review

> Source/WebCore/svg/SVGAElement.cpp:162
>      if (hasEditableStyle())
>	   return SVGGraphicsElement::supportsFocus();

How can this element be editable?

> LayoutTests/svg/custom/tabindex-order.html:66
> +    <svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink">

I think you do not have to add these namespaces in html. The xlink namespaces
is removed in SVG2 beside it is not used in this test.

> LayoutTests/svg/custom/tabindex-order.html:68
> +	   <rect class="tab" id="rect without tabindex is not focusable"
width="1" height="1"/>

I am not sure why the id's contains white spaces. I see they are not referenced
but they look weird to me. You may use comment element <!-- --> instead.