[SVG2] Add tabindex handling in svg.

Source/core/svg/SVGElement.cpp

Index: Source/core/svg/SVGElement.cpp
diff --git a/Source/core/svg/SVGElement.cpp b/Source/core/svg/SVGElement.cpp
index ca015d3ee51e2cd407a6d61be7d794ee9b1140b7..45ccad9f4e045f08db8bd1efb8caff2cb66e86a4 100644
--- a/Source/core/svg/SVGElement.cpp
+++ b/Source/core/svg/SVGElement.cpp
@@ -36,7 +36,9 @@
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/events/Event.h"
+#include "core/frame/Settings.h"
 #include "core/html/HTMLElement.h"
+#include "core/html/parser/HTMLParserIdioms.h"
 #include "core/rendering/RenderObject.h"
 #include "core/rendering/svg/RenderSVGResourceContainer.h"
 #include "core/svg/SVGCursorElement.h"
@@ -630,6 +632,49 @@ void SVGElement::setCorrespondingElement(SVGElement* correspondingElement)
     ensureSVGRareData()->setCorrespondingElement(correspondingElement);
 }
 
+short SVGElement::tabIndex() const
+{
+    if (supportsFocus())
+        return Element::tabIndex();
+    return -1;
+}
+
+void SVGElement::setTabIndex(int value)
+{
+    setIntegralAttribute(tabindexAttr, value);
+}
+
+bool SVGElement::supportsFocus() const

[pdr., 2014/03/24 16:29:54]

There's a lot of duplicate and non-obvious logic from HTMLElement here. For
example, supportsFocus, supportsSpatialNavigationFocus, and the clamping logic
in parseAttribute. Can you extract this logic into a common place?

This code may one day end up on Element. It may be worth considering moving it
there, or making that future transition as painless as we can.

[Erik Dahlström (inactive), 2014/03/25 08:24:03]

I'll try moving it up to Element, np.

+{
+    // FIXME: supportsFocus() can be called when layout is not up to date.
+    // Logic that deals with the renderer should be moved to rendererIsFocusable().
+    // But supportsFocus must return true when the element is editable, or else
+    // it won't be focusable. Furthermore, supportsFocus cannot just return true
+    // always or else tabIndex() will change for all HTML elements.
+    return Element::supportsFocus() || (rendererIsEditable() && parentNode() && !parentNode()->rendererIsEditable())
+        || supportsSpatialNavigationFocus();
+}
+
+bool SVGElement::supportsSpatialNavigationFocus() const
+{
+    // This function checks whether the element satisfies the extended criteria
+    // for the element to be focusable, introduced by spatial navigation feature,
+    // i.e. checks if click or keyboard event handler is specified.
+    // This is the way to make it possible to navigate to (focus) elements
+    // which web designer meant for being active (made them respond to click events).
+
+    if (!document().settings() || !document().settings()->spatialNavigationEnabled())
+        return false;
+    return hasEventListeners(EventTypeNames::click)
+        || hasEventListeners(EventTypeNames::keydown)
+        || hasEventListeners(EventTypeNames::keypress)
+        || hasEventListeners(EventTypeNames::keyup)
+        || hasEventListeners(EventTypeNames::focus)
+        || hasEventListeners(EventTypeNames::blur)
+        || hasEventListeners(EventTypeNames::focusin)
+        || hasEventListeners(EventTypeNames::focusout);

[Stephen Chennney, 2014/03/24 17:43:50]

Is this set spec defined? Do we need touch events or others that are typically
used for selecting something?

[Erik Dahlström (inactive), 2014/03/25 08:24:03]

It's not spec-defined no. It is mostly based on what Opera Presto did, and it's
meant to detect common scenarios where navigation is possible (e.g custom
dropdown menus and such). This functionality requires
--enable-spatial-navigation to be passed in on the commandline, it's disabled by
default.

+}
+
 void SVGElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == HTMLNames::classAttr) {
@@ -641,6 +686,19 @@ void SVGElement::parseAttribute(const QualifiedName& name, const AtomicString& v
         m_className->setBaseValueAsString(value, parseError);
         reportAttributeParsingError(parseError, name, value);
     } else if (name.matches(XMLNames::langAttr) || name.matches(XMLNames::spaceAttr)) {
+    } else if (name == tabindexAttr) {
+        int tabindex = 0;
+        if (value.isEmpty()) {
+            clearTabIndexExplicitlyIfNeeded();
+            if (treeScope().adjustedFocusedElement() == this) {
+                // We might want to call blur(), but it's dangerous to dispatch
+                // events here.
+                document().setNeedsFocusedElementCheck();
+            }
+        } else if (parseHTMLInteger(value, tabindex)) {
+            // Clamp tabindex to the range of 'short' to match Firefox's behavior.
+            setTabIndexExplicitly(max(static_cast<int>(std::numeric_limits<short>::min()), std::min(tabindex, static_cast<int>(std::numeric_limits<short>::max()))));
+        }
     } else {
         // standard events
         const AtomicString& eventName = HTMLElement::eventNameForAttributeName(name);
@@ -1025,12 +1083,8 @@ RenderStyle* SVGElement::computedStyle(PseudoId pseudoElementSpecifier)
 
 bool SVGElement::hasFocusEventListeners() const
 {
-    return hasEventListeners(EventTypeNames::focusin) || hasEventListeners(EventTypeNames::focusout);
-}
-
-bool SVGElement::isKeyboardFocusable() const
-{
-    return isFocusable();
+    return hasEventListeners(EventTypeNames::focusin) || hasEventListeners(EventTypeNames::focusout)
+        || hasEventListeners(EventTypeNames::focus) || hasEventListeners(EventTypeNames::blur);
 }
 
 #ifndef NDEBUG