Title: [158914] trunk
Revision
158914
Author
ma...@webkit.org
Date
2013-11-08 02:55:38 -0800 (Fri, 08 Nov 2013)

Log Message

AX: [ATK] <span> elements exposed through ATK when not needed
https://bugs.webkit.org/show_bug.cgi?id=123885

Reviewed by Chris Fleizach.

Source/WebCore:

As per SVN r158195, the way it's decided whether <span> elements
should be ignored or not has slightly changed, causing that the
GTK/EFL ports expose them in cases that they should be ignored,
such as for text elements that neither are focusable (e.g. by
explicitly setting tabindex) nor have a meaningful accessible name
suggesting they should be exposed.

As a result, the flattening that ATK based ports normally do for
this kind of elements (by folding them into their parents) do not
work correctly anymore, making two tests to fail:

    platform/gtk/accessibility/spans-paragraphs-and-divs.html
    platform/gtk/accessibility/spans.html

This patch encapsulates the part of the logic that affects this in
the computeAccessibilityIsIgnored() method, placing it in a
new method of AccessibilityObject that we can call from ATK's
accessibilityPlatformIncludesObject() to ensure we hide those
<span> elements when they don't fulfill those requirements.

* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::hasAttributesRequiredForInclusion):
New virtual method encapsulating part of the logic from the function
that computes whether accessibility should be ignored or not.
* accessibility/AccessibilityObject.h:

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::hasAttributesRequiredForInclusion):
Override of the new method adding additional checks, as extracted from
the original bits in computeAccessibilityIsIgnored().
* accessibility/AccessibilityNodeObject.h:

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
Use the newly added function where we had the original code before.

* accessibility/atk/AccessibilityObjectAtk.cpp:
(WebCore::AccessibilityObject::accessibilityPlatformIncludesObject):
Make sure <span> elements are ignored if they are not focusable
and they don't have a meaningful accessible name.

LayoutTests:

Removed failure expectations for tests now passing.

* platform/gtk/TestExpectations: Removed expectations.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (158913 => 158914)


--- trunk/LayoutTests/ChangeLog	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/LayoutTests/ChangeLog	2013-11-08 10:55:38 UTC (rev 158914)
@@ -1,3 +1,14 @@
+2013-11-08  Mario Sanchez Prada  <mario.pr...@samsung.com>
+
+        AX: [ATK] <span> elements exposed through ATK when not needed
+        https://bugs.webkit.org/show_bug.cgi?id=123885
+
+        Reviewed by Chris Fleizach.
+
+        Removed failure expectations for tests now passing.
+
+        * platform/gtk/TestExpectations: Removed expectations.
+
 2013-11-08  Krzysztof Czech  <k.cz...@samsung.com>
 
         [GTK] accessibility/aria-link-supports-press.html is failing

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (158913 => 158914)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2013-11-08 10:55:38 UTC (rev 158914)
@@ -1322,9 +1322,6 @@
 webkit.org/b/114259 platform/gtk/accessibility/aria-slider-required-attributes.html [ Failure ]
 webkit.org/b/114259 platform/gtk/accessibility/combo-box-collapsed-selection-changed.html [ Failure ]
 
-webkit.org/b/123885 platform/gtk/accessibility/spans-paragraphs-and-divs.html [ Failure ]
-webkit.org/b/123885 platform/gtk/accessibility/spans.html [ Failure ]
-
 webkit.org/b/114612 editing/style/block-style-005.html [ Failure ]
 
 webkit.org/b/115025 fast/events/constructors/mouse-event-constructor.html [ Failure ]

Modified: trunk/Source/WebCore/ChangeLog (158913 => 158914)


--- trunk/Source/WebCore/ChangeLog	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/ChangeLog	2013-11-08 10:55:38 UTC (rev 158914)
@@ -1,3 +1,51 @@
+2013-11-08  Mario Sanchez Prada  <mario.pr...@samsung.com>
+
+        AX: [ATK] <span> elements exposed through ATK when not needed
+        https://bugs.webkit.org/show_bug.cgi?id=123885
+
+        Reviewed by Chris Fleizach.
+
+        As per SVN r158195, the way it's decided whether <span> elements
+        should be ignored or not has slightly changed, causing that the
+        GTK/EFL ports expose them in cases that they should be ignored,
+        such as for text elements that neither are focusable (e.g. by
+        explicitly setting tabindex) nor have a meaningful accessible name
+        suggesting they should be exposed.
+
+        As a result, the flattening that ATK based ports normally do for
+        this kind of elements (by folding them into their parents) do not
+        work correctly anymore, making two tests to fail:
+
+            platform/gtk/accessibility/spans-paragraphs-and-divs.html
+            platform/gtk/accessibility/spans.html
+
+        This patch encapsulates the part of the logic that affects this in
+        the computeAccessibilityIsIgnored() method, placing it in a
+        new method of AccessibilityObject that we can call from ATK's
+        accessibilityPlatformIncludesObject() to ensure we hide those
+        <span> elements when they don't fulfill those requirements.
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::hasAttributesRequiredForInclusion):
+        New virtual method encapsulating part of the logic from the function
+        that computes whether accessibility should be ignored or not.
+        * accessibility/AccessibilityObject.h:
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::hasAttributesRequiredForInclusion):
+        Override of the new method adding additional checks, as extracted from
+        the original bits in computeAccessibilityIsIgnored().
+        * accessibility/AccessibilityNodeObject.h:
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
+        Use the newly added function where we had the original code before.
+
+        * accessibility/atk/AccessibilityObjectAtk.cpp:
+        (WebCore::AccessibilityObject::accessibilityPlatformIncludesObject):
+        Make sure <span> elements are ignored if they are not focusable
+        and they don't have a meaningful accessible name.
+
 2013-11-08  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [GTK] Add missing symbols to WebKitDOMEventTarget.symbols

Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp (158913 => 158914)


--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2013-11-08 10:55:38 UTC (rev 158914)
@@ -1914,6 +1914,17 @@
     return accessibilityDescriptionForElements(elements);
 }
 
+bool AccessibilityNodeObject::hasAttributesRequiredForInclusion() const
+{
+    if (AccessibilityObject::hasAttributesRequiredForInclusion())
+        return true;
+
+    if (!ariaAccessibilityDescription().isEmpty())
+        return true;
+
+    return false;
+}
+
 bool AccessibilityNodeObject::canSetFocusAttribute() const
 {
     Node* node = this->node();

Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h (158913 => 158914)


--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h	2013-11-08 10:55:38 UTC (rev 158914)
@@ -128,6 +128,7 @@
     virtual String stringValue() const OVERRIDE;
     virtual void colorValue(int& r, int& g, int& b) const OVERRIDE;
     virtual String ariaLabeledByAttribute() const OVERRIDE;
+    virtual bool hasAttributesRequiredForInclusion() const OVERRIDE FINAL;
 
     virtual Element* actionElement() const OVERRIDE;
     Element* mouseButtonListener() const;

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (158913 => 158914)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2013-11-08 10:55:38 UTC (rev 158914)
@@ -42,6 +42,7 @@
 #include "HTMLNames.h"
 #include "LocalizedStrings.h"
 #include "MainFrame.h"
+#include "MathMLNames.h"
 #include "NodeList.h"
 #include "NodeTraversal.h"
 #include "NotImplemented.h"
@@ -512,6 +513,23 @@
     }
 }
 
+bool AccessibilityObject::hasAttributesRequiredForInclusion() const
+{
+    // These checks are simplified in the interest of execution speed.
+    if (!getAttribute(aria_helpAttr).isEmpty()
+        || !getAttribute(aria_describedbyAttr).isEmpty()
+        || !getAttribute(altAttr).isEmpty()
+        || !getAttribute(titleAttr).isEmpty())
+        return true;
+
+#if ENABLE(MATHML)
+    if (!getAttribute(MathMLNames::alttextAttr).isEmpty())
+        return true;
+#endif
+
+    return false;
+}
+
 bool AccessibilityObject::isARIAInput(AccessibilityRole ariaRole)
 {
     return ariaRole == RadioButtonRole || ariaRole == CheckBoxRole || ariaRole == TextFieldRole;

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (158913 => 158914)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.h	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h	2013-11-08 10:55:38 UTC (rev 158914)
@@ -607,6 +607,7 @@
 
     // A programmatic way to set a name on an AccessibleObject.
     virtual void setAccessibleName(const AtomicString&) { }
+    virtual bool hasAttributesRequiredForInclusion() const;
 
     // Accessibility Text - (To be deprecated).
     virtual String accessibilityDescription() const { return String(); }

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (158913 => 158914)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2013-11-08 10:55:38 UTC (rev 158914)
@@ -1297,15 +1297,9 @@
     if (isWebArea() || isSeamlessWebArea() || m_renderer->isListMarker())
         return false;
     
-    // Using the help text, title or accessibility description (so we
-    // check if there's some kind of accessible name for the element)
-    // to decide an element's visibility is not as definitive as
-    // previous checks, so this should remain as one of the last.
-    //
-    // These checks are simplified in the interest of execution speed;
-    // for example, any element having an alt attribute will make it
-    // not ignored, rather than just images.
-    if (!getAttribute(aria_helpAttr).isEmpty() || !getAttribute(aria_describedbyAttr).isEmpty() || !getAttribute(altAttr).isEmpty() || !getAttribute(titleAttr).isEmpty())
+    // Using the presence of an accessible name to decide an element's visibility is not
+    // as definitive as previous checks, so this should remain as one of the last.
+    if (hasAttributesRequiredForInclusion())
         return false;
 
     // Don't ignore generic focusable elements like <div tabindex=0>
@@ -1313,14 +1307,6 @@
     if (isGenericFocusableElement() && node->firstChild())
         return false;
 
-    if (!ariaAccessibilityDescription().isEmpty())
-        return false;
-
-#if ENABLE(MATHML)
-    if (!getAttribute(MathMLNames::alttextAttr).isEmpty())
-        return false;
-#endif
-
     // <span> tags are inline tags and not meant to convey information if they have no other aria
     // information on them. If we don't ignore them, they may emit signals expected to come from
     // their parent. In addition, because included spans are GroupRole objects, and GroupRole

Modified: trunk/Source/WebCore/accessibility/atk/AccessibilityObjectAtk.cpp (158913 => 158914)


--- trunk/Source/WebCore/accessibility/atk/AccessibilityObjectAtk.cpp	2013-11-08 10:24:39 UTC (rev 158913)
+++ trunk/Source/WebCore/accessibility/atk/AccessibilityObjectAtk.cpp	2013-11-08 10:55:38 UTC (rev 158914)
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "AccessibilityObject.h"
 
+#include "HTMLNames.h"
 #include "RenderObject.h"
 #include "RenderText.h"
 #include <glib-object.h>
@@ -78,6 +79,19 @@
     if (role == UnknownRole)
         return IgnoreObject;
 
+    // Lines past this point only make sense for AccessibilityRenderObjects.
+    RenderObject* renderObject = renderer();
+    if (!renderObject)
+        return DefaultBehavior;
+
+    // We don't want <span> elements to show up in the accessibility hierarchy unless
+    // we have good reasons for that (e.g. focusable or visible because of containing
+    // a meaningful accessible name, maybe set through ARIA), so we can use
+    // atk_component_grab_focus() to set the focus to it.
+    Node* node = renderObject->node();
+    if (node && node->hasTagName(HTMLNames::spanTag) && !canSetFocusAttribute() && !hasAttributesRequiredForInclusion())
+        return IgnoreObject;
+
     // Given a paragraph or div containing a non-nested anonymous block, WebCore
     // ignores the paragraph or div and includes the block. We want the opposite:
     // ATs are expecting accessible objects associated with textual elements. They
@@ -86,13 +100,13 @@
     if (role == ParagraphRole || role == DivRole) {
         // Don't call textUnderElement() here, because it's slow and it can
         // crash when called while we're in the middle of a subtree being deleted.
-        if (!renderer()->firstChildSlow())
+        if (!renderObject->firstChildSlow())
             return DefaultBehavior;
 
         if (!parent->renderer() || parent->renderer()->isAnonymousBlock())
             return DefaultBehavior;
 
-        for (RenderObject* r = renderer()->firstChildSlow(); r; r = r->nextSibling()) {
+        for (RenderObject* r = renderObject->firstChildSlow(); r; r = r->nextSibling()) {
             if (r->isAnonymousBlock())
                 return IncludeObject;
         }
@@ -109,7 +123,7 @@
     // anonymous blocks which are aria-related to themselves have an aria role, nor
     // have we encountered instances where the parent of an anonymous block also lacked
     // an aria role but the grandparent had one.
-    if (renderer() && renderer()->isAnonymousBlock() && !parent->renderer()->isBody()
+    if (renderObject && renderObject->isAnonymousBlock() && !parent->renderer()->isBody()
         && parent->ariaRoleAttribute() == UnknownRole)
         return IgnoreObject;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to