- 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;