[GTK] accessibility/meter-element.html is failing
authorjdiggs@igalia.com <jdiggs@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 May 2016 18:57:33 +0000 (18:57 +0000)
committerjdiggs@igalia.com <jdiggs@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 May 2016 18:57:33 +0000 (18:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115633

Reviewed by Chris Fleizach.

Source/WebCore:

The meter's value description should be exposed in the same fashion
as (we should have been exposing) aria-valuetext, namely through the
"valuetext" AtkObject attribute. This exposure is now in place. Also
implement AccessibilityProgressIndicator::valueDescription() so that
the ports do not have to special-case meter in the platform wrappers.
Map the meter element to the correct role (ATK_ROLE_LEVEL_BAR), and
ignore a previously-included accessible object resulting from the
use of the title attribute on a meter. Finally, do not expose the
meter's title as the accessible name because the HTML spec suggests
authors can supply the numeric unit as the value of title.

No new test file as the failure was identified by meter-element.html.
Seven new test cases were added for additional coverage. Also updated
the ATK expectations for spinbutton-value.html to reflect that we are now
exposing the value of aria-valuetext.

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::helpText):
(WebCore::AccessibilityNodeObject::accessibilityDescriptionForChildren):
(WebCore::AccessibilityNodeObject::visibleText):
* accessibility/AccessibilityNodeObject.h:
* accessibility/AccessibilityProgressIndicator.cpp:
(WebCore::AccessibilityProgressIndicator::valueDescription):
* accessibility/AccessibilityProgressIndicator.h:
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
* accessibility/atk/WebKitAccessibleWrapperAtk.cpp:
(webkitAccessibleGetAttributes):
(atkRole):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):

Tools:

Implement AccessibilityUIElement::valueDescription() and add mapping
from ATK_ROLE_LEVEL_BAR to AXProgressIndicator.

* WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
(WTR::AccessibilityUIElement::valueDescription):

LayoutTests:

Seven new test cases were added to meter-element.html for additional
coverage. Also updated the ATK expectations for spinbutton-value.html
to reflect that we are now exposing the value of aria-valuetext.

* accessibility/meter-element.html: New test cases added.
* platform/gtk/TestExpectations: Unskip the previously-failing test.
* platform/gtk/accessibility/meter-element-expected.txt: Updated.
* platform/gtk/accessibility/spinbutton-value-expected.txt: Updated.
* platform/mac/accessibility/meter-element-expected.txt: Updated.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/accessibility/meter-element.html
LayoutTests/platform/gtk/TestExpectations
LayoutTests/platform/gtk/accessibility/meter-element-expected.txt
LayoutTests/platform/gtk/accessibility/spinbutton-value-expected.txt
LayoutTests/platform/mac/accessibility/meter-element-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/accessibility/AccessibilityNodeObject.h
Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp
Source/WebCore/accessibility/AccessibilityProgressIndicator.h
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp

index 96f2f99..10498c5 100644 (file)
@@ -1,3 +1,20 @@
+2016-05-18  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        [GTK] accessibility/meter-element.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=115633
+
+        Reviewed by Chris Fleizach.
+
+        Seven new test cases were added to meter-element.html for additional
+        coverage. Also updated the ATK expectations for spinbutton-value.html
+        to reflect that we are now exposing the value of aria-valuetext.
+
+        * accessibility/meter-element.html: New test cases added.
+        * platform/gtk/TestExpectations: Unskip the previously-failing test.
+        * platform/gtk/accessibility/meter-element-expected.txt: Updated.
+        * platform/gtk/accessibility/spinbutton-value-expected.txt: Updated.
+        * platform/mac/accessibility/meter-element-expected.txt: Updated.
+
 2016-05-18  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline inspector/debugger/command-line-api-exception.html after r201080
index 9469073..bce13ef 100644 (file)
 <meter id="meter5" min=0 max=10 value=2>2cm</meter>
 <meter id="meter6" min=0 max=20 value=12 title="centimeters">12cm</meter>
 <meter id="meter7" min=0 max=10 value=2 title="centimeters">2cm</meter>
+<meter id="meter8" value=0.75><img aria-label="75 out of 100" src="graph75.png"></meter>
+<meter id="meter9" value=0.75><img aria-label="75 out of 100" aria-labelledby="label" src="graph75.png"></meter>
+<meter id="meter10" value=0.75><img aria-labelledby="label" src="graph75.png"></meter>
+<span id="label" style="color:green;">75 percent</span>
+<meter id="meter11" value=0.75><img aria-labelledby="label1 label2" src="graph75.png"></meter>
+<span id="label1" style="color:green;">75</span>
+<span id="label2" style="color:green;">(100 total)</span>
+<meter id="meter12" value=0.75><span style="color:green;">75 (out of 100 total)</span></meter>
+<meter id="meter13" value=0.75><div><p><span style="color:green;">75 (out of 100 total)</span></p></div></meter>
+<meter id="meter14" value=0.60><div aria-label="7 of 10"><span style="color:green;">&#9632;&#9632;&#9632;&#9632;&#9632;&#9632;</span><span>&#9633;&#9633;&#9633;&#9633;</span></div></meter>
 </div>
 
 <div id="console"></div>
@@ -19,7 +29,7 @@
 description("This tests that the meter element is accessible.");
 
 if (window.testRunner && window.accessibilityController) {
-    for (var k = 1; k < 8; k++) {
+    for (var k = 1; k < 15; k++) {
         var meter = accessibilityController.accessibleElementById("meter" + k);
         debug("Meter" + k);
         debug(meter.role);
index 51bd946..b7618c6 100644 (file)
@@ -2099,8 +2099,6 @@ webkit.org/b/115437 fast/regions/region-content-flown-into-region.html [ ImageOn
 
 webkit.org/b/115437 fast/text/complex-initial-advance.html [ ImageOnlyFailure ]
 
-webkit.org/b/115633 accessibility/meter-element.html [ Failure ]
-
 webkit.org/b/116153 fast/css/text-overflow-ellipsis-full-truncate-rtl.html [ ImageOnlyFailure ]
 
 webkit.org/b/116806 fast/css/text-overflow-ellipsis-behind-floats.html [ ImageOnlyFailure ]
index 02b9d3d..1974cba 100644 (file)
-CONSOLE MESSAGE: line 25: TypeError: undefined is not an object (evaluating 'meter.role')
-      
 This tests that the meter element is accessible.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
 Meter1
-FAIL successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 6 blocks used (out of 8 total)
+AXValueSettable: false
+
+
+Meter2
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75%
+AXValueSettable: false
+
+
+Meter3
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 
+AXValueSettable: false
+
+
+Meter4
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 12cm
+AXValueSettable: false
+
+
+Meter5
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 2cm
+AXValueSettable: false
+
+
+Meter6
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: centimeters
+AXValueDescription: 12cm
+AXValueSettable: false
+
+
+Meter7
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: centimeters
+AXValueDescription: 2cm
+AXValueSettable: false
+
+
+Meter8
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 out of 100
+AXValueSettable: false
+
+
+Meter9
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 percent
+AXValueSettable: false
+
+
+Meter10
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 percent
+AXValueSettable: false
+
+
+Meter11
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (100 total)
+AXValueSettable: false
+
+
+Meter12
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (out of 100 total)
+AXValueSettable: false
+
+
+Meter13
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (out of 100 total)
+AXValueSettable: false
+
+
+Meter14
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 7 of 10
+AXValueSettable: false
+
+
+PASS successfullyParsed is true
 
 TEST COMPLETE
 
index a84fda2..0f0a46e 100644 (file)
@@ -8,7 +8,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 PASS endsWith(axSpin.intValue, '5') is true
 PASS endsWith(axSpin.minValue, '1') is true
 PASS endsWith(axSpin.maxValue, '9') is true
-FAIL endsWith(axSpin.valueDescription, '5 of 9') should be true. Was false.
+PASS endsWith(axSpin.valueDescription, '5 of 9') is true
 PASS axSpin.title is axUntitled.title
 PASS successfullyParsed is true
 
index 3de3774..c98166e 100644 (file)
@@ -59,6 +59,62 @@ AXValueDescription: 2cm
 AXValueSettable: false
 
 
+Meter8
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 out of 100
+AXValueSettable: false
+
+
+Meter9
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 percent
+AXValueSettable: false
+
+
+Meter10
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 percent
+AXValueSettable: false
+
+
+Meter11
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (100 total)
+AXValueSettable: false
+
+
+Meter12
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (out of 100 total)
+AXValueSettable: false
+
+
+Meter13
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 75 (out of 100 total)
+AXValueSettable: false
+
+
+Meter14
+AXRole: AXProgressIndicator
+AXTitle: 
+AXDescription: 
+AXValueDescription: 7 of 10
+AXValueSettable: false
+
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
index a58e415..de8651e 100644 (file)
@@ -1,3 +1,42 @@
+2016-05-18  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        [GTK] accessibility/meter-element.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=115633
+
+        Reviewed by Chris Fleizach.
+
+        The meter's value description should be exposed in the same fashion
+        as (we should have been exposing) aria-valuetext, namely through the
+        "valuetext" AtkObject attribute. This exposure is now in place. Also
+        implement AccessibilityProgressIndicator::valueDescription() so that
+        the ports do not have to special-case meter in the platform wrappers.
+        Map the meter element to the correct role (ATK_ROLE_LEVEL_BAR), and
+        ignore a previously-included accessible object resulting from the
+        use of the title attribute on a meter. Finally, do not expose the
+        meter's title as the accessible name because the HTML spec suggests
+        authors can supply the numeric unit as the value of title.
+
+        No new test file as the failure was identified by meter-element.html.
+        Seven new test cases were added for additional coverage. Also updated
+        the ATK expectations for spinbutton-value.html to reflect that we are now
+        exposing the value of aria-valuetext.
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::helpText):
+        (WebCore::AccessibilityNodeObject::accessibilityDescriptionForChildren):
+        (WebCore::AccessibilityNodeObject::visibleText):
+        * accessibility/AccessibilityNodeObject.h:
+        * accessibility/AccessibilityProgressIndicator.cpp:
+        (WebCore::AccessibilityProgressIndicator::valueDescription):
+        * accessibility/AccessibilityProgressIndicator.h:
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
+        * accessibility/atk/WebKitAccessibleWrapperAtk.cpp:
+        (webkitAccessibleGetAttributes):
+        (atkRole):
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
+
 2016-05-18  Antti Koivisto  <antti@apple.com>
 
         Allow RenderStyles marked unique in matched properties cache
index 4929d18..1a96098 100644 (file)
@@ -1355,7 +1355,6 @@ void AccessibilityNodeObject::visibleText(Vector<AccessibilityText>& textOrder)
     case RadioButtonRole:
     case SwitchRole:
     case TabRole:
-    case ProgressIndicatorRole:
         useTextUnderElement = true;
         break;
     default:
@@ -1396,9 +1395,17 @@ void AccessibilityNodeObject::helpText(Vector<AccessibilityText>& textOrder) con
         textOrder.append(AccessibilityText(summary, SummaryText));
 
     // The title attribute should be used as help text unless it is already being used as descriptive text.
+    // However, when the title attribute is the only text alternative provided, it may be exposed as the
+    // descriptive text. This is problematic in the case of meters because the HTML spec suggests authors
+    // can expose units through this attribute. Therefore, if the element is a meter, change its source
+    // type to HelpText.
     const AtomicString& title = getAttribute(titleAttr);
-    if (!title.isEmpty())
-        textOrder.append(AccessibilityText(title, TitleTagText));
+    if (!title.isEmpty()) {
+        if (!isMeter())
+            textOrder.append(AccessibilityText(title, TitleTagText));
+        else
+            textOrder.append(AccessibilityText(title, HelpText));
+    }
 }
 
 void AccessibilityNodeObject::accessibilityText(Vector<AccessibilityText>& textOrder)
@@ -1890,6 +1897,32 @@ static String accessibleNameForNode(Node* node, Node* labelledbyNode)
     return String();
 }
 
+String AccessibilityNodeObject::accessibilityDescriptionForChildren() const
+{
+    Node* node = this->node();
+    if (!node)
+        return String();
+
+    AXObjectCache* cache = axObjectCache();
+    if (!cache)
+        return String();
+
+    StringBuilder builder;
+    for (Node* child = node->firstChild(); child; child = child->nextSibling()) {
+        if (!is<Element>(child))
+            continue;
+
+        if (AccessibilityObject* axObject = cache->getOrCreate(child)) {
+            String description = axObject->ariaLabeledByAttribute();
+            if (description.isEmpty())
+                description = accessibleNameForNode(child);
+            appendNameToStringBuilder(builder, description);
+        }
+    }
+
+    return builder.toString();
+}
+
 String AccessibilityNodeObject::accessibilityDescriptionForElements(Vector<Element*> &elements) const
 {
     StringBuilder builder;
index 63cc7e0..6fe7143 100644 (file)
@@ -123,6 +123,7 @@ public:
 
     unsigned hierarchicalLevel() const override;
     String textUnderElement(AccessibilityTextUnderElementMode = AccessibilityTextUnderElementMode()) const override;
+    String accessibilityDescriptionForChildren() const;
     String accessibilityDescription() const override;
     String helpText() const override;
     String title() const override;
index f3a13d9..1948539 100644 (file)
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "AccessibilityProgressIndicator.h"
 
+#include "AXObjectCache.h"
 #include "FloatConversion.h"
 #include "HTMLMeterElement.h"
 #include "HTMLNames.h"
@@ -60,6 +61,36 @@ bool AccessibilityProgressIndicator::computeAccessibilityIsIgnored() const
     return accessibilityIsIgnoredByDefault();
 }
     
+String AccessibilityProgressIndicator::valueDescription() const
+{
+    // If the author has explicitly provided a value through aria-valuetext, use it.
+    String description = AccessibilityRenderObject::valueDescription();
+    if (!description.isEmpty())
+        return description;
+
+#if ENABLE(METER_ELEMENT)
+    if (!m_renderer->isMeter())
+        return String();
+
+    HTMLMeterElement* meter = meterElement();
+    if (!meter)
+        return String();
+
+    // The HTML spec encourages authors to include a textual representation of the meter's state in
+    // the element's contents. We'll fall back on that if there is not a more accessible alternative.
+    AccessibilityObject* axMeter = axObjectCache()->getOrCreate(meter);
+    if (is<AccessibilityNodeObject>(axMeter)) {
+        description = downcast<AccessibilityNodeObject>(axMeter)->accessibilityDescriptionForChildren();
+        if (!description.isEmpty())
+            return description;
+    }
+
+    return meter->textContent();
+#endif
+
+    return String();
+}
+
 float AccessibilityProgressIndicator::valueForRange() const
 {
     if (!m_renderer)
index 16e52f7..2831e5a 100644 (file)
@@ -46,6 +46,7 @@ private:
 
     bool isProgressIndicator() const override { return true; }
 
+    String valueDescription() const override;
     float valueForRange() const override;
     float maxValueForRange() const override;
     float minValueForRange() const override;
index a61a9eb..754f270 100644 (file)
@@ -1347,6 +1347,15 @@ bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
     if (isWebArea())
         return false;
     
+#if ENABLE(METER_ELEMENT)
+    // The render tree of meter includes a RenderBlock (meter) and a RenderMeter (div).
+    // We expose the latter and thus should ignore the former. However, if the author
+    // includes a title attribute on the element, hasAttributesRequiredForInclusion()
+    // will return true, potentially resulting in a redundant accessible object.
+    if (is<HTMLMeterElement>(node))
+        return true;
+#endif
+
     // 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())
index 9daff56..e106ac8 100644 (file)
@@ -420,6 +420,10 @@ static AtkAttributeSet* webkitAccessibleGetAttributes(AtkObject* object)
     if (!isReadOnly.isEmpty())
         attributeSet = addToAtkAttributeSet(attributeSet, "readonly", isReadOnly.utf8().data());
 
+    String valueDescription = coreObject->valueDescription();
+    if (!valueDescription.isEmpty())
+        attributeSet = addToAtkAttributeSet(attributeSet, "valuetext", valueDescription.utf8().data());
+
     // According to the W3C Core Accessibility API Mappings 1.1, section 5.4.1 General Rules:
     // "User agents must expose the WAI-ARIA role string if the API supports a mechanism to do so."
     // In the case of ATK, the mechanism to do so is an object attribute pair (xml-roles:"string").
@@ -514,8 +518,7 @@ static AtkRole atkRole(AccessibilityObject* coreObject)
     case BusyIndicatorRole:
         return ATK_ROLE_PROGRESS_BAR; // Is this right?
     case ProgressIndicatorRole:
-        // return ATK_ROLE_SPIN_BUTTON; // Some confusion about this role in AccessibilityRenderObject.cpp
-        return ATK_ROLE_PROGRESS_BAR;
+        return coreObject->isMeter() ? ATK_ROLE_LEVEL_BAR : ATK_ROLE_PROGRESS_BAR;
     case WindowRole:
         return ATK_ROLE_WINDOW;
     case PopUpButtonRole:
index d7b3744..045aaf0 100644 (file)
@@ -3150,12 +3150,8 @@ static NSString* roleValueToNSString(AccessibilityRole value)
         return nil;
     }
     
-    if ([attributeName isEqualToString:NSAccessibilityValueDescriptionAttribute]) {
-        if (m_object->isMeter())
-            return [self baseAccessibilityTitle];
-        
+    if ([attributeName isEqualToString:NSAccessibilityValueDescriptionAttribute])
         return m_object->valueDescription();
-    }
     
     if ([attributeName isEqualToString:NSAccessibilityOrientationAttribute]) {
         AccessibilityOrientation elementOrientation = m_object->orientation();
index 3faa204..1fa70a7 100644 (file)
@@ -1,3 +1,16 @@
+2016-05-18  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        [GTK] accessibility/meter-element.html is failing
+        https://bugs.webkit.org/show_bug.cgi?id=115633
+
+        Reviewed by Chris Fleizach.
+
+        Implement AccessibilityUIElement::valueDescription() and add mapping
+        from ATK_ROLE_LEVEL_BAR to AXProgressIndicator.
+
+        * WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
+        (WTR::AccessibilityUIElement::valueDescription):
+
 2016-05-18  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: Make TestRunner.clearAllDatabases also delete IndexedDB databases (once doing so is supported).
index ede0470..972ab58 100644 (file)
@@ -382,6 +382,8 @@ const gchar* roleToString(AtkObject* object)
         return "AXInvalid";
     case ATK_ROLE_LABEL:
         return "AXLabel";
+    case ATK_ROLE_LEVEL_BAR:
+        return "AXProgressIndicator";
     case ATK_ROLE_LINK:
         return "AXLink";
     case ATK_ROLE_LIST:
@@ -1405,8 +1407,9 @@ double AccessibilityUIElement::maxValue()
 
 JSRetainPtr<JSStringRef> AccessibilityUIElement::valueDescription()
 {
-    // FIXME: implement
-    return JSStringCreateWithCharacters(0, 0);
+    String valueText = getAttributeSetValueForId(ATK_OBJECT(m_element.get()), ObjectAttributeType, "valuetext");
+    GUniquePtr<gchar> valueDescription(g_strdup_printf("AXValueDescription: %s", valueText.utf8().data()));
+    return JSStringCreateWithUTF8CString(valueDescription.get());
 }
 
 int AccessibilityUIElement::insertionPointLineNumber()