Bug 28486 - Enable various "grouping" ARIA roles
: Enable various "grouping" ARIA roles
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Accessibility
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: chris fleizach
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-19 23:41 PDT by chris fleizach
Modified: 2009-08-21 09:21 PDT (History)
0 users

See Also:


Attachments
patch (24.00 KB, patch)
2009-08-20 00:08 PDT, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2009-08-19 23:41:39 PDT
Enable various "grouping" ARIA roles by exposing them as AXSubroles.
Comment 1 chris fleizach 2009-08-20 00:08:20 PDT
Created attachment 35187 [details]
patch

There are a large number of ARIA "grouping" type roles. On the Mac, it seems that the best fit is to make them into AXGroups with specific subroles that describe what they are
Comment 2 chris fleizach 2009-08-20 22:31:31 PDT
what happened to this positive review? darin already reviewed. also what happened to mark's comments?
Comment 3 chris fleizach 2009-08-20 22:36:23 PDT
here are marks comments

(From update of attachment 35187 [details])
+        default:
+            return nil;
+    }
+    
    return nil;
}

If you were to omit the default case in the switch statement then we'll get a
convenient warning if we ever add a new enum value without updating this switch
statement.


Index: LayoutTests/platform/mac/accessibility/aria-grouping-roles.html
===================================================================
--- LayoutTests/platform/mac/accessibility/aria-grouping-roles.html	(revision 0)
+++ LayoutTests/platform/mac/accessibility/aria-grouping-roles.html	(revision 0)
@@ -0,0 +1,56 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../../fast/js/resources/js-test-style.css">
+<script>
+var successfullyParsed = false;
+</script>
+<script src="../../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+
+<div role="application">text</div>
+<div role="article">text</div>
+<div role="banner">text</div>
+<div role="complementary">text</div>
+<div role="contentinfo">text</div>
+<div role="document">text</div>
+<div role="log">text</div>
+<div role="main">text</div>
+<div role="marquee">text</div>
+<div role="note">text</div>
+<div role="navigation">text</div>
+<div role="region">text</div>
+<div role="search">text</div>
+<div role="status">text</div>
+<div role="tooltip">text</div>

Can we give these more useful contents than "text" so that the output makes a
little more sense?  Even using just the name of the role could be clearer.

+    if (window.accessibilityController) {
+
+          // this text field should be required.
+          document.getElementById("body").focus();
+          var obj = accessibilityController.focusedElement;
+
+          var subroles = new Array("AXLandmarkApplication","AXDocumentArticle","AXLandmarkBanner","AXLandmarkComplementary",
+                          "AXLandmarkContentInfo", "AXDocument", "AXApplicationLog", "AXLandmarkMain", "AXApplicationMarquee",
+                          "AXDocumentNote", "AXLandmarkNavigation", "AXDocumentRegion", "AXLandmarkSearch", "AXApplicationStatus");
+
+          for (var k = 0; k < 14; k++) {
+              var succeeded = obj.childAtIndex(k).role == "AXRole: AXGroup" && obj.childAtIndex(k).subrole == "AXSubrole: " + subroles[k];
+              shouldBe("succeeded", "true");

The failure case of this "shouldBe" will say only that succeeded should be true
but is false, which is impossible to understand without studying the test.  I
think it would be clearer to use a separate shouldBe for each half of the
condition:
shouldBe("obj.childAtIndex(k).role", '"AXRole: AXGroup"');
shouldBe("obj.childAtIndex(k).subrole", '"AXSubrole: ' + subroles[k] + '"');

Then any failure message would be much clearer about which part of the
expression is failing and why.  It'll also make the expected result more
comprehensible.
Comment 4 chris fleizach 2009-08-20 22:37:03 PDT
Darin gave a + review at 6:00AM

Darin Adler <darin@apple.com> has granted chris fleizach
<cfleizach@apple.com>'s request for review:
Bug 28486: Enable various "grouping" ARIA roles
https://bugs.webkit.org/show_bug.cgi?id=28486
Comment 5 chris fleizach 2009-08-21 00:02:28 PDT
http://trac.webkit.org/changeset/47626
Comment 6 Eric Seidel 2009-08-21 09:21:29 PDT
Comment on attachment 35187 [details]
patch

forgetful bugzilla.