Bug 646216 - Thin out the tree by only creating accessibles for relevant divs, r=Jamie,timdream

Relevant divs are:

* Those that have an ID attribute. This is important so anchors still work.
* Those whose first or last child is a text-only node.
* Those whose first or last child has an inline frame.

We now discard divs that are not display:block; or display:inline-block;. We also discard divs that are part of an anonymous subtree.

We stop creating divs from the eHyperTextType frame type alltogether.

Note that because of shadow DOM properties in the video controls, two additional divs with IDs require role="none" in the media controls widget code.

Differential Revision: https://phabricator.services.mozilla.com/D17348

--HG--
extra : moz-landing-system : lando

accessible/base/MarkupMap.h

@@ -57,7 +57,66 @@ MARKUPMAP(del, New_HyperText, roles::CONTENT_DELETION)
 
 MARKUPMAP(details, New_HyperText, roles::DETAILS)
 
-MARKUPMAP(div, nullptr, roles::SECTION)
+MARKUPMAP(
+    div,
+    [](Element* aElement, Accessible* aContext) -> Accessible* {
+      // Never create an accessible if we're part of an anonymous
+      // subtree.
+      if (aElement->IsInAnonymousSubtree()) {
+        return nullptr;
+      }
+      // Always create an accessible if the div has an id.
+      if (aElement->HasAttr(kNameSpaceID_None, nsGkAtoms::id)) {
+        return new HyperTextAccessibleWrap(aElement, aContext->Document());
+      }
+      // Never create an accessible if the div is not display:block; or
+      // display:inline-block;
+      nsAutoString displayValue;
+      StyleInfo styleInfo(aElement);
+      styleInfo.Display(displayValue);
+      if (displayValue != NS_LITERAL_STRING("block") &&
+          displayValue != NS_LITERAL_STRING("inline-block")) {
+        return nullptr;
+      }
+      // Check for various conditions to determine if this is a block
+      // break and needs to be rendered.
+      // If its previous sibling is an inline element, we probably want
+      // to break, so render.
+      nsIContent* prevSibling = aElement->GetPreviousSibling();
+      if (prevSibling) {
+        nsIFrame* prevSiblingFrame = prevSibling->GetPrimaryFrame();
+        if (prevSiblingFrame && prevSiblingFrame->IsInlineFrame()) {
+          return new HyperTextAccessibleWrap(aElement, aContext->Document());
+        }
+      }
+      // Now, check the children.
+      nsIContent* firstChild = aElement->GetFirstChild();
+      if (firstChild) {
+        // Render it if it is a text node.
+        if (firstChild->IsText()) {
+          return new HyperTextAccessibleWrap(aElement, aContext->Document());
+        }
+        // Check to see if first child has an inline frame.
+        nsIFrame* firstChildFrame = firstChild->GetPrimaryFrame();
+        if (firstChildFrame && firstChildFrame->IsInlineFrame()) {
+          return new HyperTextAccessibleWrap(aElement, aContext->Document());
+        }
+        nsIContent* lastChild = aElement->GetLastChild();
+        if (lastChild && lastChild != firstChild) {
+          // Render it if it is a text node.
+          if (lastChild->IsText()) {
+            return new HyperTextAccessibleWrap(aElement, aContext->Document());
+          }
+          // Check to see if last child has an inline frame.
+          nsIFrame* lastChildFrame = lastChild->GetPrimaryFrame();
+          if (lastChildFrame && lastChildFrame->IsInlineFrame()) {
+            return new HyperTextAccessibleWrap(aElement, aContext->Document());
+          }
+        }
+      }
+      return nullptr;
+    },
+    roles::SECTION)
 
 MARKUPMAP(dl,
           [](Element* aElement, Accessible* aContext) -> Accessible* {

accessible/base/nsAccessibilityService.cpp

@@ -20,6 +20,7 @@
 #include "HTMLTableAccessibleWrap.h"
 #include "HyperTextAccessibleWrap.h"
 #include "RootAccessible.h"
+#include "StyleInfo.h"
 #include "nsAccUtils.h"
 #include "nsArrayUtils.h"
 #include "nsAttrName.h"
@@ -1408,8 +1409,10 @@ nsAccessibilityService::CreateAccessibleByFrameType(nsIFrame* aFrame,
       newAcc = new HTMLTextFieldAccessible(aContent, document);
       break;
     case eHyperTextType:
-      if (!aContent->IsAnyOfHTMLElements(nsGkAtoms::dt, nsGkAtoms::dd))
+      if (!aContent->IsAnyOfHTMLElements(nsGkAtoms::dt, nsGkAtoms::dd,
+                                         nsGkAtoms::div)) {
         newAcc = new HyperTextAccessibleWrap(aContent, document);
+      }
       break;
 
     case eImageType:

accessible/tests/mochitest/events/test_text.html

@@ -124,6 +124,10 @@
 
       this.invoke = function insertChildDiv_invoke() {
         var childDiv = document.createElement("div");
+        // Note after bug 646216, a sole div without text won't be accessible
+        // and would not result in an embedded character.
+        // Therefore, add some text.
+        childDiv.textContent = "hello";
         this.DOMNode.insertBefore(childDiv, this.DOMNode.childNodes[1]);
       };
 

accessible/tests/mochitest/states/test_stale.html

@@ -28,6 +28,10 @@
 
       this.invoke = function addChild_invoke() {
         this.childNode = document.createElement("div");
+        // Note after bug 646216, a sole div without text won't be accessible
+        // and would not result in an embedded character.
+        // Therefore, add some text.
+        this.childNode.textContent = "hello";
         this.containerNode.appendChild(this.childNode);
       };
 

accessible/tests/mochitest/tree/a11y.ini

@@ -26,6 +26,7 @@ skip-if = true # Bug 561508
 [test_cssflexbox.html]
 [test_cssoverflow.html]
 [test_display_contents.html]
+[test_divs.html]
 [test_dochierarchy.html]
 [test_dockids.html]
 [test_filectrl.html]

accessible/tests/mochitest/tree/test_divs.html

@@ -0,0 +1,235 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+<title>div element creation tests</title>
+<link rel="stylesheet" type="text/css"
+      href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+<script type="application/javascript"
+        src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+<script type="application/javascript"
+        src="../common.js"></script>
+<script type="application/javascript"
+        src="../role.js"></script>
+<script type="application/javascript"
+        src="../attributes.js"></script>
+
+<script type="application/javascript">
+function doTest() {
+  // All below test cases are wrapped in a div which always gets rendered.
+  // c1 through c10 are the containers, the actual test cases are inside.
+
+  // c1: Expose the div with text content
+  let tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // inner div
+          children: [
+            { TEXT_LEAF: [] },
+          ], // end children inner div
+        }, // end inner div
+      ], // end children outer div
+    };
+  testAccessibleTree("c1", tree);
+
+  // c2: Only the outermost and innermost divs are exposed.
+  // The middle one is skipped. This is identical to the above tree.
+  testAccessibleTree("c2", tree);
+
+  // c3: Make sure the inner div with ID is exposed, but the middle one is
+  // skipped.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // inner div
+          attributes: { id: "b" },
+          children: [
+            { TEXT_LEAF: [] },
+          ], // end children inner div
+        }, // end inner div
+      ], // end children outer div
+    };
+  testAccessibleTree("c3", tree);
+
+  // c4: Expose all three divs including the middle one due to its ID.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // middle div
+          attributes: { id: "a" },
+          children: [
+            { role: ROLE_SECTION, // inner div
+              attributes: { id: "b" },
+              children: [
+                { TEXT_LEAF: [] },
+              ], // end children inner div
+            }, // end inner div
+          ], // end children middle div
+        }, // end middle div
+      ], // end children outer div
+    };
+  testAccessibleTree("c4", tree);
+
+  // c5: Expose the inner div with class b due to its text contents.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // inner div with class and text
+          attributes: { class: "b" },
+          children: [
+            { TEXT_LEAF: [] },
+          ], // end children inner div
+        }, // end inner div
+      ], // end children outer div
+    };
+  testAccessibleTree("c5", tree);
+
+  // c6: Expose the outer div due to its ID, and the two inner divs due to
+  // their text contents. Skip the middle one.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // first inner div
+          children: [
+            { TEXT_LEAF: [] },
+          ], // end children first inner div
+        }, // end first inner div
+        { role: ROLE_SECTION, // second inner div
+          children: [
+            { TEXT_LEAF: [] },
+          ], // end children second inner div
+        }, // end second inner div
+      ], // end children outer div
+    };
+  testAccessibleTree("c6", tree);
+
+  // c7: Expose all three divs including the middle one due to it being block
+  // breaking.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // middle div
+          children: [
+            { TEXT_LEAF: [] }, // foo
+            { role: ROLE_SECTION, // inner div
+              children: [
+                { TEXT_LEAF: [] }, // bar
+              ], // end children inner div
+            }, // end inner div
+            { TEXT_LEAF: [] }, // baz
+          ], // end children middle div
+        }, // end middle div
+      ], // end children outer div
+    };
+  testAccessibleTree("c7", tree);
+
+  // c8: Expose all divs due to them all being text block breakers.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // foo div
+          children: [
+            { TEXT_LEAF: [] }, // foo
+            { role: ROLE_SECTION, // baz div
+              children: [
+                { role: ROLE_SECTION, // bar div
+                  children: [
+                    { TEXT_LEAF: [] }, // bar
+                  ], // end children bar div
+                }, // end bar div
+                { TEXT_LEAF: [] }, // baz
+              ], // end children baz div
+            }, // end baz div
+          ], // end children foo div
+        }, // end foo div
+      ], // end children outer div
+    };
+  testAccessibleTree("c8", tree);
+
+  // c9: The same, but in a different nesting order.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // c div
+          children: [
+            { role: ROLE_SECTION, // b div
+              children: [
+                { role: ROLE_SECTION, // a div
+                  children: [
+                    { TEXT_LEAF: [] }, // a
+                  ], // end children a div
+                }, // end a div
+                { TEXT_LEAF: [] }, // b
+              ], // end children b div
+            }, // end b div
+            { TEXT_LEAF: [] }, // c
+          ], // end children c div
+        }, // end foo div
+      ], // end children outer div
+    };
+  testAccessibleTree("c9", tree);
+
+  // c10: Both inner divs must be exposed so there is a break after b.
+  tree =
+    { role: ROLE_SECTION, // outer div with ID
+      children: [
+        { role: ROLE_SECTION, // first inner div
+          children: [
+            { TEXT_LEAF: [] }, // a
+            { TEXT_LEAF: [] }, // b
+          ], // end children first inner div
+        }, // end first inner div
+        { role: ROLE_SECTION, // second inner div
+          children: [
+            { TEXT_LEAF: [] }, // c
+            { TEXT_LEAF: [] }, // d
+          ], // end children second inner div
+        }, // end second inner div
+      ], // end children outer div
+    };
+  testAccessibleTree("c10", tree);
+
+  SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+addA11yLoadEvent(doTest);
+</script>
+</head>
+<body>
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <!-- Expose the div if it has plain text contents -->
+  <div id="c1"><div>foo</div></div>
+
+  <!-- Expose the outer and inner div, skip the middle one. -->
+  <div id="c2"><div><div>foo</div></div></div>
+
+  <!-- Expose the outer and inner divs due to the ID, but skip the middle one. -->
+  <div id="c3"><div><div id="b">foo</div></div></div>
+
+  <!-- Expose all three divs and their IDs. -->
+  <div id="c4"><div id="a"><div id="b">foo</div></div></div>
+
+  <!-- Expose outer and inner divs, due to text content, not class. -->
+  <div id="c5"><div class="a"><div class="b">foo</div></div></div>
+
+  <!-- Expose the outer and two inner divs, skip the single middle one. -->
+  <div id="c6"><div><div>foo</div><div>bar</div></div></div>
+
+  <!-- Expose all divs due to the middle one being block breaking. -->
+  <div id="c7"><div>foo<div>bar</div>baz</div></div>
+
+  <!-- Expose all divs due to them all being text block breakers. -->
+  <div id="c8"><div>foo<div><div>bar</div>baz</div></div></div>
+  <div id="c9"><div><div><div>a</div>b</div>c</div></div>
+
+  <!-- Both inner divs need to be rendered so there is a break after b. -->
+  <div id="c10"><div><b>a</b>b</div><div><b>c</b><b>d</b></div></div>
+</body>
+</html>

accessible/tests/mochitest/tree/test_list.html

@@ -151,21 +151,17 @@
       // dl with div grouping dt/dd
       tree =
         { DEFINITION_LIST: [ // dl
-          { SECTION: [ // div
+          { TERM: [ // dt
-            { TERM: [ // dt
+            { TEXT_LEAF: [] },
-              { TEXT_LEAF: [] },
-            ] },
-            { DEFINITION: [ // dd
-              { TEXT_LEAF: [] },
-            ] },
           ] },
-          { SECTION: [ // div
+          { DEFINITION: [ // dd
-            { TERM: [ // dt
+            { TEXT_LEAF: [] },
-              { TEXT_LEAF: [] },
+          ] },
-            ] },
+          { TERM: [ // dt
-            { DEFINITION: [ // dd
+            { TEXT_LEAF: [] },
-              { TEXT_LEAF: [] },
+          ] },
-            ] },
+          { DEFINITION: [ // dd
+            { TEXT_LEAF: [] },
           ] },
         ] };
 

accessible/tests/mochitest/treeupdate/test_cssoverflow.html

@@ -13,6 +13,8 @@
           src="../common.js"></script>
   <script type="application/javascript"
           src="../role.js"></script>
+  <script type="application/javascript"
+          src="../states.js"></script>
   <script type="application/javascript"
           src="../events.js"></script>
 
@@ -60,8 +62,8 @@
     }
 
     /**
-     * Change scrollbar styles from hidden to auto. That makes us to create an
+     * Change scrollbar styles from hidden to auto. That causes us to create a
-     * accessible for scroll area.
+     * new accessible for scroll area.
      */
     function changeScrollbarStyles(aContainerID, aScrollAreaID) {
       this.container = getAccessible(aContainerID);
@@ -74,7 +76,9 @@
 
       this.invoke = function changeScrollbarStyles_invoke() {
         var accTree =
-          { SECTION: [] };
+          { SECTION: [ // container
+            { SECTION: [] }, // scroll area
+          ] };
         testAccessibleTree(this.container, accTree);
 
         this.scrollAreaNode.style.overflow = "auto";
@@ -93,6 +97,54 @@
       };
     }
 
+    /**
+     * Change scrollbar styles from hidden to auto to make the scroll area focusable.
+     * That causes us to create an accessible for it.
+     * Make sure the tree stays intact.
+     * The scroll area has no ID on purpose to make it inaccessible initially.
+     */
+    function makeFocusableByScrollbarStyles(aContainerID) {
+      this.container = getAccessible(aContainerID);
+      this.scrollAreaNode = getNode(aContainerID).firstChild;
+
+      this.eventSeq = [
+        new invokerChecker(EVENT_SHOW, getAccessible, this.scrollAreaNode),
+        new invokerChecker(EVENT_REORDER, this.container),
+      ];
+
+      this.invoke = function makeFocusableByScrollbarStyles_invoke() {
+        var accTree =
+          { SECTION: [ // container
+            { PARAGRAPH: [ // paragraph
+              { TEXT_LEAF: [] },
+            ] },
+          ] };
+        testAccessibleTree(this.container, accTree);
+
+        this.scrollAreaNode.style.overflow = "auto";
+      };
+
+      this.finalCheck = function makeFocusableByScrollbarStyles_finalCheck() {
+        var accTree =
+          { SECTION: [ // container
+            { role: ROLE_SECTION,  // focusable scroll area
+              states: STATE_FOCUSABLE,
+              children: [
+                { PARAGRAPH: [ // paragraph
+                  { TEXT_LEAF: [] }, // text leaf
+                ] },
+              ],
+            }, // focusable scroll area
+          ] };
+        testAccessibleTree(this.container, accTree);
+      };
+
+      this.getID = function makeFocusableByScrollbarStyles_getID() {
+        return "make div focusable through scrollbar styles "
+               + prettyName(aContainerID);
+      };
+    }
+
     // //////////////////////////////////////////////////////////////////////////
     // Do tests
     // //////////////////////////////////////////////////////////////////////////
@@ -106,6 +158,7 @@
 
       gQueue.push(new changeScrollRange("container", "scrollarea"));
       gQueue.push(new changeScrollbarStyles("container2", "scrollarea2"));
+      gQueue.push(new makeFocusableByScrollbarStyles("container3"));
 
       gQueue.invoke(); // Will call SimpleTest.finish();
     }
@@ -130,5 +183,6 @@
 
   <div id="container"><div id="scrollarea" style="overflow:auto;"><input></div></div>
   <div id="container2"><div id="scrollarea2" style="overflow:hidden;"></div></div>
+  <div id="container3"><div style="overflow: hidden; height: 1px;"><p>foo</p></div></div>
 </body>
 </html>

toolkit/content/widgets/videocontrols.js

@@ -2210,13 +2210,13 @@ this.VideoControlsImplWidget = class {
             <span class="errorLabel" id="errorGeneric">&error.generic;</span>
           </div>
 
-          <div id="controlsOverlay" class="controlsOverlay stackItem">
+          <div id="controlsOverlay" class="controlsOverlay stackItem" role="none">
             <div class="controlsSpacerStack">
               <div id="controlsSpacer" class="controlsSpacer stackItem" role="none"></div>
               <div id="clickToPlay" class="clickToPlay" hidden="true"></div>
             </div>
 
-            <div id="controlBar" class="controlBar" hidden="true">
+            <div id="controlBar" class="controlBar" role="none" hidden="true">
               <button id="playButton"
                       class="button playButton"
                       playlabel="&playButton.playLabel;"