SVGElement::isMouseFocusable() has bizarre code which checks the presence of keydown, keyup, keypress, focusin, focusout, focus, and blur event listeners. We shouldn't make an element focusable just because it has a bunch of event listeners.
Created attachment 376945 [details] Patch
Comment on attachment 376945 [details] 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.
(In reply to Said Abou-Hallawa from comment #2) > Comment on attachment 376945 [details] > 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? You can put contenteditble attribute anywhere, or put the entire document into design mode by document.designMode = 'on'. > > 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. Yeah, I added for some testing. Removed. > > 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. Well, these IDs are like that because if they ever get focused, it would show up in the test result. The rest of the test uses that convention so I'd keep that for now.
Thanks for the review!
Committed r248983: <https://trac.webkit.org/changeset/248983>
<rdar://problem/54579978>
*** Bug 140024 has been marked as a duplicate of this bug. ***