From 09554268a2d2241f5ec11ef52a83f8f9a0a1b312 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Thu, 8 Aug 2019 16:47:14 +0000 Subject: [PATCH] Bug 1571616 - Prune or reinsert accessibles in non-accessible container. r=Jamie We need to visit the descendants of a container that has no accessible, but has accessible children. Also added a test for delayed removal where we can put all these nasty cases. Differential Revision: https://phabricator.services.mozilla.com/D41059 --HG-- extra : moz-landing-system : lando --- accessible/generic/DocAccessible.cpp | 21 +- .../tests/mochitest/treeupdate/a11y.ini | 1 + .../treeupdate/test_delayed_removal.html | 219 ++++++++++++++++++ .../mochitest/treeupdate/test_general.html | 93 -------- 4 files changed, 235 insertions(+), 99 deletions(-) create mode 100644 accessible/tests/mochitest/treeupdate/test_delayed_removal.html diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index 7aa7d623d3f3..fe14d999e416 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -1308,6 +1308,7 @@ void DocAccessible::ContentInserted(nsIContent* aStartChildNode, } bool DocAccessible::PruneOrInsertSubtree(nsIContent* aRoot) { + bool insert = false; // If we already have an accessible, check if we need to remove it, recreate // it, or keep it in place. Accessible* acc = GetAccessible(aRoot); @@ -1349,7 +1350,19 @@ bool DocAccessible::PruneOrInsertSubtree(nsIContent* aRoot) { // If there is no current accessible, and the node has a frame, or is // display:contents, schedule it for insertion. if (aRoot->GetPrimaryFrame() || nsCoreUtils::IsDisplayContents(aRoot)) { - return true; + // This may be a new subtree, the insertion process will recurse through + // its descendants. + if (!GetAccessibleOrDescendant(aRoot)) { + return true; + } + + // Content is not an accessible, but has accessible descendants. + // We schedule this container for insertion strictly for the case where it + // itself now needs an accessible. We will still need to recurse into the + // descendant content to prune accessibles, and in all likelyness to + // insert accessibles since accessible insertions will likeley get missed + // in an existing subtree. + insert = true; } } @@ -1368,11 +1381,7 @@ bool DocAccessible::PruneOrInsertSubtree(nsIContent* aRoot) { } } - // If we get here we either already have an accessible we don't want to touch, - // or the content does not have a frame and is not display:contents. - MOZ_ASSERT(acc || (!aRoot->GetPrimaryFrame() && - !nsCoreUtils::IsDisplayContents(aRoot))); - return false; + return insert; } void DocAccessible::RecreateAccessible(nsIContent* aContent) { diff --git a/accessible/tests/mochitest/treeupdate/a11y.ini b/accessible/tests/mochitest/treeupdate/a11y.ini index 959e70aaeacd..8234b5a1a581 100644 --- a/accessible/tests/mochitest/treeupdate/a11y.ini +++ b/accessible/tests/mochitest/treeupdate/a11y.ini @@ -21,6 +21,7 @@ support-files = test_bug1276857_subframe.html [test_contextmenu.xul] [test_cssoverflow.html] [test_deck.xul] +[test_delayed_removal.html] [test_doc.html] [test_gencontent.html] [test_general.html] diff --git a/accessible/tests/mochitest/treeupdate/test_delayed_removal.html b/accessible/tests/mochitest/treeupdate/test_delayed_removal.html new file mode 100644 index 000000000000..740e0e779153 --- /dev/null +++ b/accessible/tests/mochitest/treeupdate/test_delayed_removal.html @@ -0,0 +1,219 @@ + + + + + Test accessible delayed removal + + + + + + + + + + + + + + + +

+ + 
+
+ +
+
hello
 +
+ +
+ +
+ +
+
hello
 +
+ +
text
+ +
text
+ +
+
+
+
+
+ +
+
+ +
+
+ +
+ + diff --git a/accessible/tests/mochitest/treeupdate/test_general.html b/accessible/tests/mochitest/treeupdate/test_general.html index 7279df8a567a..5a2f1462ce1d 100644 --- a/accessible/tests/mochitest/treeupdate/test_general.html +++ b/accessible/tests/mochitest/treeupdate/test_general.html @@ -133,93 +133,6 @@ }; } - // This tests a case where a container (c5) gets re-framed - // because an inline child has its block child set to display: none. - function hideBlockChildOfInline() { - this.eventSeq = [ - new invokerChecker(EVENT_HIDE, "div"), - new invokerChecker(EVENT_REORDER, "span"), - ]; - - this.invoke = function hideBlockChildOfInline_invoke() { - document.body.offsetTop; // Flush layout. - - getNode("div").style.display = "none"; - - }; - - this.finalCheck = function hideBlockChildOfInline_finalCheck() { - var accTree = - { SECTION: [ // container - { REGION: [] }, - ] }; - testAccessibleTree("c5", accTree); - }; - - this.getID = function hideBlockChildOfInline_getID() { - return "hide div from inside span."; - }; - } - - // This tests a case where a container (c5) gets re-framed - // because an inline child has its block child set to display: none. - function reinsertBlockChildOfInline() { - this.eventSeq = [ - new invokerChecker(EVENT_REORDER, "span"), - ]; - - this.invoke = function reinsertBlockChildOfInline_invoke() { - document.body.offsetTop; // Flush layout. - - getNode("div").style.display = "block"; - - }; - - this.finalCheck = function reinsertBlockChildOfInline_finalCheck() { - var accTree = - { SECTION: [ // container - { REGION: [ - { SECTION: [ - { TEXT_LEAF: [] } - ] } - ] }, - ] }; - testAccessibleTree("c5", accTree); - }; - - this.getID = function reinsertBlockChildOfInline_getID() { - return "reinsert div from inside span."; - }; - } - - // This tests a case where a container (c5) gets re-framed - // because an inline child has its block child removed. - function removeBlockChildOfInline() { - this.eventSeq = [ - new invokerChecker(EVENT_HIDE, getNode("div")), - new invokerChecker(EVENT_REORDER, "span"), - ]; - - this.invoke = function removeBlockChildOfInline_invoke() { - document.body.offsetTop; // Flush layout. - - getNode("div").remove(); - - }; - - this.finalCheck = function removeBlockChildOfInline_finalCheck() { - var accTree = - { SECTION: [ // container - { REGION: [] }, - ] }; - testAccessibleTree("c5", accTree); - }; - - this.getID = function removeBlockChildOfInline_getID() { - return "remove div from inside span."; - }; - } - // ////////////////////////////////////////////////////////////////////////// // Do tests @@ -236,9 +149,6 @@ gQueue.push(new removeRemove("c2")); gQueue.push(new insertInaccessibleAccessibleSiblings()); gQueue.push(new displayContentsInsertion()); - gQueue.push(new hideBlockChildOfInline()); - gQueue.push(new reinsertBlockChildOfInline()); - gQueue.push(new removeBlockChildOfInline()); gQueue.invoke(); // Will call SimpleTest.finish(); } @@ -259,9 +169,6 @@
-
-
hello
 -