From 0b5d5b3e24bf3e1d80daeb0bf9eb1f521006c7be Mon Sep 17 00:00:00 2001 From: Alexander Surkov Date: Fri, 19 Feb 2016 13:11:33 -0500 Subject: [PATCH] Bug 1248838 - ARIA owns change may fail, r=yzen --- accessible/generic/Accessible.cpp | 45 ++++++++++++++++++ accessible/generic/Accessible.h | 5 ++ accessible/generic/DocAccessible.cpp | 11 +++-- accessible/tests/mochitest/common.js | 21 +++++++++ .../mochitest/treeupdate/test_ariaowns.html | 46 +++++++++++++++++-- 5 files changed, 119 insertions(+), 9 deletions(-) diff --git a/accessible/generic/Accessible.cpp b/accessible/generic/Accessible.cpp index 42dc1978d7e2..34c119e8521f 100644 --- a/accessible/generic/Accessible.cpp +++ b/accessible/generic/Accessible.cpp @@ -2130,6 +2130,51 @@ Accessible::RemoveChild(Accessible* aChild) return true; } +void +Accessible::MoveChild(uint32_t aNewIndex, Accessible* aChild) +{ + MOZ_ASSERT(aChild, "No child was given"); + MOZ_ASSERT(aChild->mParent == this, "A child from different subtree was given"); + MOZ_ASSERT(aChild->mIndexInParent != -1, "Unbound child was given"); + MOZ_ASSERT(static_cast(aChild->mIndexInParent) != aNewIndex, + "No move, same index"); + MOZ_ASSERT(aNewIndex <= mChildren.Length(), "Wrong new index was given"); + +#ifdef DEBUG + // AutoTreeMutation should update group info. + AssertInMutatingSubtree(); +#endif + + mEmbeddedObjCollector = nullptr; + mChildren.RemoveElementAt(aChild->mIndexInParent); + + uint32_t startIdx = aNewIndex, endIdx = aChild->mIndexInParent; + + // If the child is moved after its current position. + if (static_cast(aChild->mIndexInParent) < aNewIndex) { + startIdx = aChild->mIndexInParent; + + if (aNewIndex == mChildren.Length() + 1) { + // The child is moved to the end. + mChildren.AppendElement(aChild); + endIdx = mChildren.Length() - 1; + } + else { + mChildren.InsertElementAt(aNewIndex - 1, aChild); + endIdx = aNewIndex; + } + } + else { + // The child is moved prior its current position. + mChildren.InsertElementAt(aNewIndex, aChild); + } + + for (uint32_t idx = startIdx; idx <= endIdx; idx++) { + mChildren[idx]->mIndexInParent = idx; + mChildren[idx]->mInt.mIndexOfEmbeddedChild = -1; + } +} + Accessible* Accessible::GetChildAt(uint32_t aIndex) const { diff --git a/accessible/generic/Accessible.h b/accessible/generic/Accessible.h index 788ff3d2904f..2317aee5d000 100644 --- a/accessible/generic/Accessible.h +++ b/accessible/generic/Accessible.h @@ -396,6 +396,11 @@ public: virtual bool InsertChildAt(uint32_t aIndex, Accessible* aChild); virtual bool RemoveChild(Accessible* aChild); + /** + * Reallocates the child withing its parent. + */ + void MoveChild(uint32_t aNewIndex, Accessible* aChild); + ////////////////////////////////////////////////////////////////////////////// // Accessible tree traverse methods diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index 3c9e675a5907..92da3de28b60 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -2058,7 +2058,10 @@ DocAccessible::DoARIAOwnsRelocation(Accessible* aOwner) } if (child->Parent() == aOwner) { - MoveChild(child, insertIdx - 1); + if (child->IsRelocated()) { + children->RemoveElement(child); + } + MoveChild(child, insertIdx); children->InsertElementAt(arrayIdx, child); arrayIdx++; @@ -2141,9 +2144,7 @@ DocAccessible::MoveChild(Accessible* aChild, int32_t aIdxInParent) reorderEvent->AddSubMutationEvent(hideEvent); AutoTreeMutation mut(parent); - parent->RemoveChild(aChild); - - parent->InsertChildAt(aIdxInParent, aChild); + parent->MoveChild(aIdxInParent, aChild); aChild->SetRelocated(true); FireDelayedEvent(hideEvent); @@ -2174,6 +2175,7 @@ DocAccessible::PutChildrenBack(nsTArray >* aChildren, RefPtr reorderEvent = new AccReorderEvent(owner); RefPtr hideEvent = new AccHideEvent(child, false); reorderEvent->AddSubMutationEvent(hideEvent); + FireDelayedEvent(hideEvent); { AutoTreeMutation mut(owner); @@ -2181,7 +2183,6 @@ DocAccessible::PutChildrenBack(nsTArray >* aChildren, child->SetRelocated(false); } - FireDelayedEvent(hideEvent); MaybeNotifyOfValueChange(owner); FireDelayedEvent(reorderEvent); } diff --git a/accessible/tests/mochitest/common.js b/accessible/tests/mochitest/common.js index 40166436d547..842af97d03a0 100644 --- a/accessible/tests/mochitest/common.js +++ b/accessible/tests/mochitest/common.js @@ -110,6 +110,27 @@ function isLogged(aModule) return gAccRetrieval.isLogged(aModule); } +/** + * Dumps the accessible tree into console. + */ +function dumpTree(aId, aMsg) +{ + function dumpTreeIntl(acc, indent) + { + dump(indent + prettyName(acc) + "\n"); + + var children = acc.children; + for (var i = 0; i < children.length; i++) { + var child = children.queryElementAt(i, nsIAccessible); + dumpTreeIntl(child, indent + " "); + } + } + + dump(aMsg + "\n"); + var root = getAccessible(aId); + dumpTreeIntl(root, " "); +} + /** * Invokes the given function when document is loaded and focused. Preferable * to mochitests 'addLoadEvent' function -- additionally ensures state of the diff --git a/accessible/tests/mochitest/treeupdate/test_ariaowns.html b/accessible/tests/mochitest/treeupdate/test_ariaowns.html index b0ea67b9cc88..2070136f1b78 100644 --- a/accessible/tests/mochitest/treeupdate/test_ariaowns.html +++ b/accessible/tests/mochitest/treeupdate/test_ariaowns.html @@ -23,19 +23,21 @@ // Invokers //////////////////////////////////////////////////////////////////////////// - function removeARIAOwns() + function changeARIAOwns() { this.eventSeq = [ - new invokerChecker(EVENT_HIDE, getNode("t1_checkbox")), new invokerChecker(EVENT_HIDE, getNode("t1_button")), new invokerChecker(EVENT_SHOW, getNode("t1_button")), + new invokerChecker(EVENT_HIDE, getNode("t1_subdiv")), + new invokerChecker(EVENT_SHOW, getNode("t1_subdiv")), + new invokerChecker(EVENT_HIDE, getNode("t1_checkbox")), new invokerChecker(EVENT_SHOW, getNode("t1_checkbox")), new invokerChecker(EVENT_REORDER, getNode("t1_container")) ]; - this.invoke = function removeARIAOwns_invoke() + this.invoke = function setARIAOwns_invoke() { - // children are swapped + // children are swapped by ARIA owns var tree = { SECTION: [ { CHECKBUTTON: [ @@ -45,6 +47,41 @@ ] }; testAccessibleTree("t1_container", tree); + getNode("t1_container"). + setAttribute("aria-owns", "t1_button t1_subdiv"); + } + + this.finalCheck = function setARIAOwns_finalCheck() + { + // children are swapped again, button and subdiv are appended to + // the children. + var tree = + { SECTION: [ + { CHECKBUTTON: [ ] }, // checkbox, native order + { PUSHBUTTON: [ ] }, // button, rearranged by ARIA own + { SECTION: [ ] } // subdiv from the subtree, ARIA owned + ] }; + testAccessibleTree("t1_container", tree); + } + + this.getID = function setARIAOwns_getID() + { + return "Change @aria-owns attribute"; + } + } + + function removeARIAOwns() + { + this.eventSeq = [ + new invokerChecker(EVENT_HIDE, getNode("t1_button")), + new invokerChecker(EVENT_HIDE, getNode("t1_subdiv")), + new invokerChecker(EVENT_SHOW, getNode("t1_button")), + new invokerChecker(EVENT_SHOW, getNode("t1_subdiv")), + new invokerChecker(EVENT_REORDER, getNode("t1_container")) + ]; + + this.invoke = function removeARIAOwns_invoke() + { getNode("t1_container").removeAttribute("aria-owns"); } @@ -418,6 +455,7 @@ gQueue = new eventQueue(); // test1 + gQueue.push(new changeARIAOwns()); gQueue.push(new removeARIAOwns()); gQueue.push(new setARIAOwns()); gQueue.push(new addIdToARIAOwns());