There is a discussion about adding the "tabindex" attribute to SVG content (see http://lists.w3.org/Archives/Public/www-svg/2012Jul/0108.html). This is a lot easier to understand (and use) for author used to HTML. There is also the "focusable" attribute in SVG 1.2Tiny (cover by bug 409404) but it's not clear at the moment which attribute will be promote in SVG2, see http://www.w3.org/Graphics/SVG/WG/wiki/SVG2_Requirements_Input#Run-time_Synchronization_attributes IMO, it would be good to implement both attributes : "focusable" for backward compatibility with legacy content, "tabindex" to make authors' life easier in the future when they'll mixe both technologies.
Requests coming in from the wild - perhaps the time is now. Jonathan, Neil, who would be the right person to work on this? How much effort do you think it would take?
You would just need to implement nsIContent::IsFocusable on the desired elements and set the aTtabIndex argument as necessary within that implementation.
OK! Maybe I could just tweak the SVGAElement::IsFocusable code that morphs tabindex to -1...
Status: I wrote a half-baked patch and then this totally fell off my radar. Note I would still like to hear from jwatt.
I think we should only implement 'tabindex', at least for now. SVG Tiny has not generally been used on the public Web since of the desktop browsers only Opera implemented it I believe. Regarding the a/b/c options in heycam's email (linked in the first paragraph of comment 0), I'd defer to heycam on that.
Rich went with duplicating tabIndex, focus and blur on SVGElement (option c), and I think that's fine.
Okay, fine by me too. Thanks, Cameron.
Jonathan do you know who could pick up this work?
I don't think anyone is likely to be able to get to it before the end of the year. I potentially could in January, but check with my manager (Jet) regarding his priorities and how that would fit in.
Jonathan, any progress on this? Tabindex in the SVG 2 specificaton: https://svgwg.org/svg2-draft/interact.html#sequential-focus-navigation-and-the-tabindex-attribute I also added event handlers to be in line with HTML5 which started with focus: https://svgwg.org/svg2-draft/interact.html#SVGEvents The Document interface also includes support for activeElement: https://svgwg.org/svg2-draft/struct.html#InterfaceSVGDocument The SVG elements also support focus() and blur() There is also support for onfocusin, onfocusout, onfocus, and onblur event handlers: https://svgwg.org/svg2-draft/script.html#GraphicsEvents
Erik Dalhstrom has tabindex under code review now before it drops in Canary. So, it will be in Chrome Canary soon.
Thanks Rik.
(In reply to David Bolter [:davidb] from comment #3) > OK! > > Maybe I could just tweak the SVGAElement::IsFocusable code that morphs > tabindex to -1... I'm experimenting with isFocusable, but that function is never called for an SVG file. Do you have an example that uses this functionality?
(In reply to Rik Cabanier from comment #13) > (In reply to David Bolter [:davidb] from comment #3) > I'm experimenting with isFocusable, but that function is never called for an > SVG file. > Do you have an example that uses this functionality? Not sure if this works for you. http://people.mozilla.org/~dbolter/svg-focus.html
I had tried adding this to SVGAElement::IsFocusable: + if (HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)) { + return true; + } And somewhere else... sorry I've forgotten...
Created attachment 8400172 [details] [diff] [review] 1. Add IDL for tabindex
Friendly 2 week review nudge for Cameron.
Created attachment 8407288 [details] [diff] [review] tabindexidl.patch
*** Bug 996407 has been marked as a duplicate of this bug. ***
Comment on attachment 8407288 [details] [diff] [review] tabindexidl.patch Review of attachment 8407288 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay. Some high level comments to start with. I think the patch needs to be split up into two pieces: one implements 'outline' for SVG elements, the other does the focus related things. ::: content/svg/content/src/nsSVGElement.cpp @@ +1145,5 @@ > return mClassAttribute.ToDOMAnimatedString(this); > } > > +Nullable<int32_t> > +nsSVGElement::GetTabIndex() Is there a way we can share code between these four methods you've added here and the ones that exist on nsGenericHTMLElement? They basically should have the same behaviour. ::: content/svg/content/test/reftest/reftest.list @@ +1,3 @@ > +# SVG reftests > + > +pref(svg.tabindex.enabled,true) == tabindex.html tabindex-ref.html \ No newline at end of file Reftests should go in layout/svg/reftests/. ::: dom/webidl/SVGElement.webidl @@ +26,5 @@ > > readonly attribute SVGSVGElement? ownerSVGElement; > readonly attribute SVGElement? viewportElement; > + > + [Pref="svg.tabindex.enabled"] attribute long? tabIndex; Why is this nullable? That doesn't match what's in the spec. ::: layout/svg/svg.css @@ +85,5 @@ > +polyline:-moz-focusring, > +rect:-moz-focusring, > +text:-moz-focusring, > +use:-moz-focusring, > +image:-moz-focusring I think this should be something like: :-moz-focusring:not(:root) so that all SVG elements that would render can have an outline, except for the root element.
Comment on attachment 8407288 [details] [diff] [review] tabindexidl.patch Review of attachment 8407288 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/svg/content/src/nsSVGElement.cpp @@ +1145,5 @@ > return mClassAttribute.ToDOMAnimatedString(this); > } > > +Nullable<int32_t> > +nsSVGElement::GetTabIndex() The way to do that would be to put the functionality in their base class but it might not be the correct place for it and I'm wary to make such a big change. ::: content/svg/content/test/reftest/reftest.list @@ +1,3 @@ > +# SVG reftests > + > +pref(svg.tabindex.enabled,true) == tabindex.html tabindex-ref.html \ No newline at end of file Fixed ::: dom/webidl/SVGElement.webidl @@ +26,5 @@ > > readonly attribute SVGSVGElement? ownerSVGElement; > readonly attribute SVGElement? viewportElement; > + > + [Pref="svg.tabindex.enabled"] attribute long? tabIndex; I don't remember why I did this. Fixed ::: layout/svg/svg.css @@ +85,5 @@ > +polyline:-moz-focusring, > +rect:-moz-focusring, > +text:-moz-focusring, > +use:-moz-focusring, > +image:-moz-focusring wouldn't that include items such as <symbol> and <gradient>? I thought that we were going to call these elements out from the spec. Maybe we can pick up the email threat again?
Created attachment 8408763 [details] [diff] [review] 1. Add DispayOutline object to tree for SVG elements that can be focused
Created attachment 8408764 [details] [diff] [review] 2. Add IDL + support for tabindex + focus to SVG elements. Also added tests.
Created attachment 8409277 [details] [diff] [review] 2. Add IDL + support for tabindex + focus to SVG elements. Also added tests.
try run: https://tbpl.mozilla.org/?tree=Try&rev=f0bbb14ecd65
Please move the code to a common base class so we don't have two copies.
Is this code in the nightly builds now?
(In reply to Rich Schwerdtfeger from comment #27) > Is this code in the nightly builds now? not yet, it's still requires review. Maybe somebody else should do it because the current reviewer doesn't look active.
My review comment not to create two copies of the code still needs addressing. If that's done Cam or I will rereview.
Rick is comment 29 something you agree with and can address?
(In reply to David Bolter [:davidb] from comment #30) > Rick is comment 29 something you agree with and can address? I will try to get to it this week. At the time I thought it might be overkill to do it this way but I will investigate.
It looks like this one's pretty close to resolution. Is there anything we could do to help it move forward?
(In reply to duane.obrien from comment #32) > It looks like this one's pretty close to resolution. Is there anything we > could do to help it move forward? I would like to finish it but keep getting pulled into other things :-( Would you have time to get it done?
It's not something I could finish unfortunately, I'm just tracking the bug against another project that's waiting for it.
Are we any closer to resolution on this one?
This is part of the SVG2 specification. It needs to be implemented. It is mostly there in Webkit and Chrome.
I think you should only implement tabindex for now. There is an effort that has started called the SVG Accessibility Task Force and we will at some point consider keyboard navigation mechanisms.
Rik, do you have status update on this bug? Do you still need review/needinfo from cam?
Any new information on this one?