Bug 509696 - Empty role on body removes read-only state; breaks virtual buffers. r=MarcoZ,surkov
authorDavid Bolter <dbolter@mozilla.com>
Mon, 31 Aug 2009 09:12:08 -0400
changeset 32104 6aab4d7bb538132d266e2609abe0103804da5f1c
parent 32103 e7e7d401fd8d7ed7ea29e9609f0f89bd622ce5fb
child 32105 f2ebd467b1cdd427d62ccf0433416ea0b8173414
push id8849
push userdbolter@mozilla.com
push dateMon, 31 Aug 2009 13:12:57 +0000
treeherderautoland@6aab4d7bb538 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMarcoZ, surkov
bugs509696
milestone1.9.3a1pre
Bug 509696 - Empty role on body removes read-only state; breaks virtual buffers. r=MarcoZ,surkov
accessible/src/base/nsAccUtils.cpp
accessible/src/base/nsAccessible.cpp
accessible/tests/mochitest/test_states_doc.html
--- a/accessible/src/base/nsAccUtils.cpp
+++ b/accessible/src/base/nsAccUtils.cpp
@@ -643,17 +643,20 @@ nsAccUtils::GetScreenCoordsForParent(nsI
   return nsIntPoint(parentRect.x, parentRect.y);
 }
 
 nsRoleMapEntry*
 nsAccUtils::GetRoleMapEntry(nsIDOMNode *aNode)
 {
   nsIContent *content = nsCoreUtils::GetRoleContent(aNode);
   nsAutoString roleString;
-  if (!content || !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::role, roleString)) {
+  if (!content ||
+      !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::role, roleString) ||
+      roleString.IsEmpty()) {
+    // We treat role="" as if the role attribute is absent (per aria spec:8.1.1)
     return nsnull;
   }
 
   nsWhitespaceTokenizer tokenizer(roleString);
   while (tokenizer.hasMoreTokens()) {
     // Do a binary search through table for the next role in role list
     NS_LossyConvertUTF16toASCII role(tokenizer.nextToken());
     PRUint32 low = 0;
@@ -669,17 +672,17 @@ nsAccUtils::GetRoleMapEntry(nsIDOMNode *
         high = index;
       }
       else {
         low = index + 1;
       }
     }
   }
 
-  // Always use some entry if there is a role string
+  // Always use some entry if there is a non-empty role string
   // To ensure an accessible object is created
   return &nsARIAMap::gLandmarkRoleMap;
 }
 
 PRUint32
 nsAccUtils::RoleInternal(nsIAccessible *aAcc)
 {
   PRUint32 role = nsIAccessibleRole::ROLE_NOTHING;
--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@@ -2025,19 +2025,22 @@ nsAccessible::GetARIAState(PRUint32 *aSt
 
   PRUint32 index = 0;
   while (nsStateMapEntry::MapToStates(content, aState, aExtraState,
                                       nsARIAMap::gWAIUnivStateMap[index])) {
     ++ index;
   }
 
   if (mRoleMapEntry) {
-    // Once an ARIA role is used, default to not-readonly. This can be overridden
-    // by aria-readonly, or if the ARIA role is mapped to readonly by default
-    *aState &= ~nsIAccessibleStates::STATE_READONLY;
+
+    // We only force the readonly bit off if we have a real mapping for the aria
+    // role. This preserves the ability for screen readers to use readonly
+    // (primarily on the document) as the hint for creating a virtual buffer.
+    if (mRoleMapEntry->role != nsIAccessibleRole::ROLE_NOTHING)
+      *aState &= ~nsIAccessibleStates::STATE_READONLY;
 
     if (content->HasAttr(kNameSpaceID_None, content->GetIDAttributeName())) {
       // If has a role & ID and aria-activedescendant on the container, assume focusable
       nsIContent *ancestorContent = content;
       while ((ancestorContent = ancestorContent->GetParent()) != nsnull) {
         if (ancestorContent->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::aria_activedescendant)) {
             // ancestor has activedescendant property, this content could be active
           *aState |= nsIAccessibleStates::STATE_FOCUSABLE;
--- a/accessible/tests/mochitest/test_states_doc.html
+++ b/accessible/tests/mochitest/test_states_doc.html
@@ -1,13 +1,14 @@
 <!DOCTYPE html>
 <html>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=454997
 https://bugzilla.mozilla.org/show_bug.cgi?id=467387
+https://bugzilla.mozilla.org/show_bug.cgi?id=509696
 -->
 <head>
   <title>states of document</title>
   <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
 
   <script type="application/javascript"
           src="chrome://mochikit/content/MochiKit/packed.js"></script>
   <script type="application/javascript"
@@ -16,16 +17,29 @@ https://bugzilla.mozilla.org/show_bug.cg
   <script type="application/javascript"
           src="chrome://mochikit/content/a11y/accessible/common.js"></script>
   <script type="application/javascript"
           src="chrome://mochikit/content/a11y/accessible/states.js"></script>
 
   <script type="application/javascript">
     function doTest()
     {
+      // Bug 509696
+      testStates(document, STATE_READONLY); // role=""
+      todo(false, "enable commented tests when we support body role changes");
+/*
+      document.body.setAttribute("role","banner"); // no platform mapping
+      testStates(document, STATE_READONLY);
+      document.body.setAttribute("role","foo"); // bogus role
+      testStates(document, STATE_READONLY);
+      document.body.removeAttribute("role");
+      testStates(document, STATE_READONLY);
+*/
+      
+      // Bugs 454997 and 467387
       testStates(document, STATE_READONLY);
       testStates("document", STATE_READONLY);
       testStates("editable_document", 0, EXT_STATE_EDITABLE);
 
       document.designMode = "on";
 
       testStates(document, 0, EXT_STATE_EDITABLE);
       testStates("p", 0, EXT_STATE_EDITABLE);
@@ -41,24 +55,27 @@ https://bugzilla.mozilla.org/show_bug.cg
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addLoadEvent(doTest);
   </script>
 </head>
 
-<body>
+<body role="">
 
   <a target="_blank"
-     title="nsIAccessible states tests of editable document"
+     title="<body contenteditable='true'> exposed incorrectly"
      href="https://bugzilla.mozilla.org/show_bug.cgi?id=454997">Mozilla Bug 454997</a>
    <a target="_blank"
      title="nsIAccessible states tests of editable document"
      href="https://bugzilla.mozilla.org/show_bug.cgi?id=467387">Mozilla Bug 467387</a>
+   <a target="_blank"
+     title="Role attribute on body with empty string causes DocAccessible not to have read-only state."
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=509696">Mozilla Bug 509696</a>
   <p id="display"></p>
   <div id="content" style="display: none"></div>
   <pre id="test">
   </pre>
 
   <p id="p">hello</p>
 
   <div id="document" role="document">document</div>