SVG element should become focusable when focus and key event listeners are added
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Aug 2019 00:34:48 +0000 (00:34 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Aug 2019 00:34:48 +0000 (00:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200997

Reviewed by Said Abou-Hallawa.

Source/WebCore:

This patch removes the odd behavior WebKit (and Blink) browsers had to make SVG elements
with key or focus event listeners focusable. New behavior matches the behavior of Firefox
as well as the SVG 2.0 specification: https://www.w3.org/TR/SVG2/interact.html#Focus

Test: svg/custom/tabindex-order.html

* svg/SVGAElement.cpp:
(WebCore::SVGAElement::supportsFocus const):
* svg/SVGElement.cpp:
(WebCore::SVGElement::hasFocusEventListeners const): Deleted.
(WebCore::SVGElement::isMouseFocusable const): Deleted.
* svg/SVGElement.h:
* svg/SVGGraphicsElement.h:

LayoutTests:

Updated existing tests to set tabIndex where appropriate, and added SVG elements
without tabindex content attribute to tabindex-order.html so that the test would
skip those elements when sequentially focus navigating across them.

* svg/custom/add-event-listener-shadow-tree-element.html:
* svg/custom/resources/focus-event-handling-keyboard.js:
* svg/custom/resources/focus-event-handling.js:
* svg/custom/tabindex-order-expected.txt:
* svg/custom/tabindex-order.html: Added test cases without tabindex.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248983 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/custom/add-event-listener-shadow-tree-element.html
LayoutTests/svg/custom/resources/focus-event-handling-keyboard.js
LayoutTests/svg/custom/resources/focus-event-handling.js
LayoutTests/svg/custom/tabindex-order-expected.txt
LayoutTests/svg/custom/tabindex-order.html
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGAElement.cpp
Source/WebCore/svg/SVGElement.cpp
Source/WebCore/svg/SVGElement.h
Source/WebCore/svg/SVGGraphicsElement.h

index 57929d1..7a424b7 100644 (file)
@@ -1,3 +1,20 @@
+2019-08-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        SVG element should become focusable when focus and key event listeners are added
+        https://bugs.webkit.org/show_bug.cgi?id=200997
+
+        Reviewed by Said Abou-Hallawa.
+
+        Updated existing tests to set tabIndex where appropriate, and added SVG elements
+        without tabindex content attribute to tabindex-order.html so that the test would
+        skip those elements when sequentially focus navigating across them.
+
+        * svg/custom/add-event-listener-shadow-tree-element.html:
+        * svg/custom/resources/focus-event-handling-keyboard.js:
+        * svg/custom/resources/focus-event-handling.js:
+        * svg/custom/tabindex-order-expected.txt:
+        * svg/custom/tabindex-order.html: Added test cases without tabindex.
+
 2019-08-21  Megan Gardner  <megan_gardner@apple.com>
 
         Do not adjust viewport if editing selection is already visible
index 9ffdc91..251c5af 100644 (file)
@@ -19,6 +19,7 @@
                 testRunner.dumpAsText();
                 testRunner.waitUntilDone();
             }
+            use1.tabIndex = 0;
             use1.setAttribute("onfocusin", "eventhandler()");
             use1.focus();
         }
index ff147a1..5d6f99a 100644 (file)
@@ -22,16 +22,22 @@ function focusoutHandler(evt)
     focusoutSeen = evt.target.getAttribute('id');
 }
 
+rectElement.tabIndex = 0;
 rectElement.setAttribute("onfocusin", "focusinHandler(evt)");
 rectElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+gElement.tabIndex = 0;
 gElement.setAttribute("onfocusin", "focusinHandler(evt)");
 gElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+useElement.tabIndex = 0;
 useElement.setAttribute("onfocusin", "focusinHandler(evt)");
 useElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+useElement2.tabIndex = 0;
 useElement2.setAttribute("onfocusin", "focusinHandler(evt)");
 useElement2.setAttribute("onfocusout", "focusoutHandler(evt)");
+switchElement.tabIndex = 0;
 switchElement.setAttribute("onfocusin", "focusinHandler(evt)");
 switchElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+imgElement.tabIndex = 0;
 imgElement.setAttribute("onfocusin", "focusinHandler(evt)");
 imgElement.setAttribute("onfocusout", "focusoutHandler(evt)");
 
index 4c70620..01a33a6 100644 (file)
@@ -28,16 +28,22 @@ function focusoutHandler(evt)
     focusoutSeen = evt.target.getAttribute('id');
 }
 
+rectElement.tabIndex = 0;
 rectElement.setAttribute("onfocusin", "focusinHandler(evt)");
 rectElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+gElement.tabIndex = 0;
 gElement.setAttribute("onfocusin", "focusinHandler(evt)");
 gElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+useElement.tabIndex = 0;
 useElement.setAttribute("onfocusin", "focusinHandler(evt)");
 useElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+useElement2.tabIndex = 0;
 useElement2.setAttribute("onfocusin", "focusinHandler(evt)");
 useElement2.setAttribute("onfocusout", "focusoutHandler(evt)");
+switchElement.tabIndex = 0;
 switchElement.setAttribute("onfocusin", "focusinHandler(evt)");
 switchElement.setAttribute("onfocusout", "focusoutHandler(evt)");
+imgElement.tabIndex = 0;
 imgElement.setAttribute("onfocusin", "focusinHandler(evt)");
 imgElement.setAttribute("onfocusout", "focusoutHandler(evt)");
 
index dcca2be..086c6b8 100644 (file)
@@ -7,6 +7,7 @@ To test, put focus in "a". Pressing Tab should focus "a" through "k" in order, a
 id: a tabindex: 1 [object SVGCircleElement] is focused.
 id: b tabindex: 1 [object SVGGElement] is focused.
 id: c tabindex: 1 [object SVGEllipseElement] is focused.
+id: symbol tabindex: 1 [object SVGSymbolElement] is focused.
 id: d tabindex: 1 [object SVGPathElement] is focused.
 id: e tabindex: 3 [object SVGAElement] is focused.
 id: f tabindex: 4 [object SVGPolylineElement] is focused.
@@ -26,6 +27,7 @@ id: g tabindex: 6 [object SVGRectElement] is focused.
 id: f tabindex: 4 [object SVGPolylineElement] is focused.
 id: e tabindex: 3 [object SVGAElement] is focused.
 id: d tabindex: 1 [object SVGPathElement] is focused.
+id: symbol tabindex: 1 [object SVGSymbolElement] is focused.
 id: c tabindex: 1 [object SVGEllipseElement] is focused.
 id: b tabindex: 1 [object SVGGElement] is focused.
 id: a tabindex: 1 [object SVGCircleElement] is focused.
index 44e17ca..b9fd8cc 100644 (file)
     <p>To test, put focus in &quot;a&quot;. Pressing Tab should focus &quot;a&quot; through &quot;k&quot; in order, and pressing Shift-Tab should reverse the order.</p>
     <svg>
         <rect class="tab" tabindex="6" id="g" width="1" height="1"/>
+        <rect class="tab" id="rect without tabindex is not focusable" width="1" height="1"/>
         <circle class="tab" tabindex="1" id="a" r="1" cx="0" cy="0"/>
+        <circle class="tab" id="circle without tabindex is not focusable" r="1" cx="0" cy="0"/>
         <rect class="tab" tabindex="-5" id="not in tab order (negative tabindex)" width="1" height="1"/>
         <g class="tab" tabindex="1" id="b"/>
+        <g class="tab" id="g without tabindex is not focusable"/>
         <svg class="tab" tabindex="0" id="i" width="1" height="1"/>
         <text class="tab" tabindex="6" id="h" width="1" height="1"/>
+        <text class="tab" id="text without tabindex is not focusable" width="1" height="1"/>
         <ellipse class="tab" tabindex="1" id="c" rx="1" ry="1" cx="0" cy="0"/>
-        <symbol class="tab" tabindex="1" id="symbol is not focusable" width="1" height="1"/>
+        <ellipse class="tab" id="ellipse without tabindex is not focusable" rx="1" ry="1" cx="0" cy="0"/>
+        <symbol class="tab" tabindex="1" id="symbol" width="1" height="1"/>
+        <symbol class="tab" id="symbol without tabindex is not focusable" width="1" height="1"/>
         <defs class="tab" tabindex="1" id="defs is not focusable"/>
         <path class="tab" tabindex="1" id="d" d="M0,0"/>
+        <path class="tab" id="path without tabindex is not focusable" d="M0,0"/>
         <line class="tab" tabindex="0" id="j" x1="1" x2="1" y1="0" y2="0"/>
+        <line class="tab" id="line without tabindex is not focusable" x1="1" x2="1" y1="0" y2="0"/>
         <rect class="tab" tabindex="-1" id="not in tab order (negative tabindex)" width="1" height="1"/>
         <polygon class="tab" tabindex="0" id="k" points="1,1 2,2"/>
+        <polygon class="tab" id="polygon without tabindex is not focusable" points="1,1 2,2"/>
         <polyline class="tab" tabindex="4" id="f" points="1,1 2,2"/>
+        <polyline class="tab" id="polyline without tabindex is not focusable" points="1,1 2,2"/>
         <a class="tab" tabindex="3" id="e"><rect width="1" height="1"/></a>
     </svg>
 
index bbe73d6..2641cc4 100644 (file)
@@ -1,3 +1,24 @@
+2019-08-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        SVG element should become focusable when focus and key event listeners are added
+        https://bugs.webkit.org/show_bug.cgi?id=200997
+
+        Reviewed by Said Abou-Hallawa.
+
+        This patch removes the odd behavior WebKit (and Blink) browsers had to make SVG elements
+        with key or focus event listeners focusable. New behavior matches the behavior of Firefox
+        as well as the SVG 2.0 specification: https://www.w3.org/TR/SVG2/interact.html#Focus
+
+        Test: svg/custom/tabindex-order.html
+
+        * svg/SVGAElement.cpp:
+        (WebCore::SVGAElement::supportsFocus const):
+        * svg/SVGElement.cpp:
+        (WebCore::SVGElement::hasFocusEventListeners const): Deleted.
+        (WebCore::SVGElement::isMouseFocusable const): Deleted.
+        * svg/SVGElement.h:
+        * svg/SVGGraphicsElement.h:
+
 2019-08-21  Jer Noble  <jer.noble@apple.com>
 
         Adopt AVSystemController_ActiveAudioRouteDidChangeNotification
index a4d15fe..81ab424 100644 (file)
@@ -161,7 +161,7 @@ bool SVGAElement::supportsFocus() const
     if (hasEditableStyle())
         return SVGGraphicsElement::supportsFocus();
     // If not a link we should still be able to focus the element if it has a tabIndex.
-    return isLink() || Element::supportsFocus();
+    return isLink() || SVGGraphicsElement::supportsFocus();
 }
 
 bool SVGAElement::isURLAttribute(const Attribute& attribute) const
index a94a918..38c8f8d 100644 (file)
@@ -983,26 +983,6 @@ void SVGElement::updateRelativeLengthsInformation(bool hasRelativeLengths, SVGEl
     }
 }
 
-bool SVGElement::hasFocusEventListeners() const
-{
-    Element* eventTarget = const_cast<SVGElement*>(this);
-    return eventTarget->hasEventListeners(eventNames().focusinEvent)
-        || eventTarget->hasEventListeners(eventNames().focusoutEvent)
-        || eventTarget->hasEventListeners(eventNames().focusEvent)
-        || eventTarget->hasEventListeners(eventNames().blurEvent);
-}
-
-bool SVGElement::isMouseFocusable() const
-{
-    if (!isFocusable())
-        return false;
-    Element* eventTarget = const_cast<SVGElement*>(this);
-    return hasFocusEventListeners()
-        || eventTarget->hasEventListeners(eventNames().keydownEvent)
-        || eventTarget->hasEventListeners(eventNames().keyupEvent)
-        || eventTarget->hasEventListeners(eventNames().keypressEvent);
-}
-    
 void SVGElement::accessKeyAction(bool sendMouseEvents)
 {
     dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents);
index 9546256..f36db5d 100644 (file)
@@ -116,7 +116,6 @@ public:
 
     bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions&) override;
     bool removeEventListener(const AtomString& eventType, EventListener&, const ListenerOptions&) override;
-    bool hasFocusEventListeners() const;
 
     bool hasTagName(const SVGQualifiedName& name) const { return hasLocalName(name.localName()); }
 
@@ -153,9 +152,6 @@ protected:
     SVGElement(const QualifiedName&, Document&);
     virtual ~SVGElement();
 
-    bool isMouseFocusable() const override;
-    bool supportsFocus() const override { return false; }
-
     bool rendererIsNeeded(const RenderStyle&) override;
     void parseAttribute(const QualifiedName&, const AtomString&) override;
 
index a4755ad..425c327 100644 (file)
@@ -70,8 +70,6 @@ public:
 protected:
     SVGGraphicsElement(const QualifiedName&, Document&);
 
-    bool supportsFocus() const override { return Element::supportsFocus() || hasFocusEventListeners(); }
-
     void parseAttribute(const QualifiedName&, const AtomString&) override;
     void svgAttributeChanged(const QualifiedName&) override;