From 5ebe7949aebd315f937ed34d63d734fcbaf18e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 18 Oct 2018 10:02:51 +0200 Subject: [PATCH] Bug 1487312 - Fix content insertion accessibility notifications in Shadow DOM. r=Jamie,surkov The issue was specific to content insertion directly under a shadow root, the rest should work (see bug 1427825 for the fix for other similar occurrences). The removal of the aContainer argument follows the same pattern as bug 1442207. Differential Revision: https://phabricator.services.mozilla.com/D6431 --- accessible/base/NotificationController.cpp | 34 +++++++++++++------ accessible/base/NotificationController.h | 3 +- accessible/base/nsAccessibilityService.cpp | 9 ++--- accessible/generic/DocAccessible.cpp | 23 ++++--------- accessible/generic/DocAccessible.h | 4 +-- .../mochitest/treeupdate/test_bug1276857.html | 6 +++- 6 files changed, 39 insertions(+), 40 deletions(-) diff --git a/accessible/base/NotificationController.cpp b/accessible/base/NotificationController.cpp index e2e1a76bd0c4..b4a2d752ec56 100644 --- a/accessible/base/NotificationController.cpp +++ b/accessible/base/NotificationController.cpp @@ -417,27 +417,39 @@ NotificationController::ScheduleChildDocBinding(DocAccessible* aDocument) } void -NotificationController::ScheduleContentInsertion(Accessible* aContainer, - nsIContent* aStartChildNode, +NotificationController::ScheduleContentInsertion(nsIContent* aStartChildNode, nsIContent* aEndChildNode) { - nsTArray> list; + // The frame constructor guarantees that only ranges with the same parent + // arrive here in presence of dynamic changes to the page, see + // nsCSSFrameConstructor::IssueSingleInsertNotifications' callers. + nsINode* parent = aStartChildNode->GetFlattenedTreeParentNode(); + if (!parent) { + return; + } - bool needsProcessing = false; - nsIContent* node = aStartChildNode; - while (node != aEndChildNode) { + Accessible* container = mDocument->AccessibleOrTrueContainer(parent); + if (!container) { + return; + } + + AutoTArray, 10> list; + for (nsIContent* node = aStartChildNode; + node != aEndChildNode; + node = node->GetNextSibling()) { + MOZ_ASSERT(parent == node->GetFlattenedTreeParentNode()); // Notification triggers for content insertion even if no content was // actually inserted, check if the given content has a frame to discard // this case early. + // + // TODO(emilio): Should this handle display: contents? if (node->GetPrimaryFrame()) { - if (list.AppendElement(node)) - needsProcessing = true; + list.AppendElement(node); } - node = node->GetNextSibling(); } - if (needsProcessing) { - mContentInsertions.LookupOrAdd(aContainer)->AppendElements(list); + if (!list.IsEmpty()) { + mContentInsertions.LookupOrAdd(container)->AppendElements(list); ScheduleProcessing(); } } diff --git a/accessible/base/NotificationController.h b/accessible/base/NotificationController.h index 60684fba78ed..cdb10ff0ace7 100644 --- a/accessible/base/NotificationController.h +++ b/accessible/base/NotificationController.h @@ -195,8 +195,7 @@ public: /** * Pend accessible tree update for content insertion. */ - void ScheduleContentInsertion(Accessible* aContainer, - nsIContent* aStartChildNode, + void ScheduleContentInsertion(nsIContent* aStartChildNode, nsIContent* aEndChildNode); /** diff --git a/accessible/base/nsAccessibilityService.cpp b/accessible/base/nsAccessibilityService.cpp index 4b84a1e39e58..1f17c66b436a 100644 --- a/accessible/base/nsAccessibilityService.cpp +++ b/accessible/base/nsAccessibilityService.cpp @@ -385,10 +385,7 @@ nsAccessibilityService::ListenersChanged(nsIArray* aEventChanges) // handler. if (document && !document->HasAccessible(node) && nsCoreUtils::HasClickListener(node)) { - nsIContent* parentEl = node->GetFlattenedTreeParent(); - if (parentEl) { - document->ContentInserted(parentEl, node, node->GetNextSibling()); - } + document->ContentInserted(node, node->GetNextSibling()); break; } } @@ -611,7 +608,7 @@ nsAccessibilityService::DeckPanelSwitched(nsIPresShell* aPresShell, } #endif - document->ContentInserted(aDeckNode, panelNode, panelNode->GetNextSibling()); + document->ContentInserted(panelNode, panelNode->GetNextSibling()); } } @@ -635,7 +632,7 @@ nsAccessibilityService::ContentRangeInserted(nsIPresShell* aPresShell, #endif if (document) { - document->ContentInserted(aStartChild->GetParent(), aStartChild, aEndChild); + document->ContentInserted(aStartChild, aEndChild); } } diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index a4d5b89b322d..ff41b2751330 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -777,10 +777,8 @@ DocAccessible::AttributeChanged(dom::Element* aElement, if (aAttribute == nsGkAtoms::aria_hidden) { if (aria::HasDefinedARIAHidden(aElement)) { ContentRemoved(aElement); - } - else { - ContentInserted(aElement->GetFlattenedTreeParent(), - aElement, aElement->GetNextSibling()); + } else { + ContentInserted(aElement, aElement->GetNextSibling()); } return; } @@ -1353,8 +1351,7 @@ DocAccessible::UnbindFromDocument(Accessible* aAccessible) } void -DocAccessible::ContentInserted(nsIContent* aContainerNode, - nsIContent* aStartChildNode, +DocAccessible::ContentInserted(nsIContent* aStartChildNode, nsIContent* aEndChildNode) { // Ignore content insertions until we constructed accessible tree. Otherwise @@ -1362,14 +1359,8 @@ DocAccessible::ContentInserted(nsIContent* aContainerNode, if (mNotificationController && HasLoadState(eTreeConstructed)) { // Update the whole tree of this document accessible when the container is // null (document element is inserted or removed). - Accessible* container = aContainerNode ? - AccessibleOrTrueContainer(aContainerNode) : this; - if (container) { - // Ignore notification if the container node is no longer in the DOM tree. - mNotificationController->ScheduleContentInsertion(container, - aStartChildNode, - aEndChildNode); - } + mNotificationController->ScheduleContentInsertion(aStartChildNode, + aEndChildNode); } } @@ -1388,10 +1379,8 @@ DocAccessible::RecreateAccessible(nsIContent* aContent) // subclass hide and show events to handle them separately and implement their // coalescence with normal hide and show events. Note, in this case they // should be coalesced with normal show/hide events. - - nsIContent* parent = aContent->GetFlattenedTreeParent(); ContentRemoved(aContent); - ContentInserted(parent, aContent, aContent->GetNextSibling()); + ContentInserted(aContent, aContent->GetNextSibling()); } void diff --git a/accessible/generic/DocAccessible.h b/accessible/generic/DocAccessible.h index 345bd22c2479..fe372c19f10d 100644 --- a/accessible/generic/DocAccessible.h +++ b/accessible/generic/DocAccessible.h @@ -350,9 +350,7 @@ public: /** * Notify the document accessible that content was inserted. */ - void ContentInserted(nsIContent* aContainerNode, - nsIContent* aStartChildNode, - nsIContent* aEndChildNode); + void ContentInserted(nsIContent* aStartChildNode, nsIContent* aEndChildNode); /** * Update the tree on content removal. diff --git a/accessible/tests/mochitest/treeupdate/test_bug1276857.html b/accessible/tests/mochitest/treeupdate/test_bug1276857.html index 0db7ad51ab7c..aa10b4158664 100644 --- a/accessible/tests/mochitest/treeupdate/test_bug1276857.html +++ b/accessible/tests/mochitest/treeupdate/test_bug1276857.html @@ -81,13 +81,17 @@ var shadowRoot = iframe.contentDocument.getElementById("c2_c").shadowRoot; shadowRoot.firstElementChild.querySelector("span").remove(); + // bug 1487312 + shadowRoot.firstElementChild.offsetTop; + shadowRoot.appendChild(document.createElement("button")); }; this.finalCheck = function runShadowTest_finalCheck() { var tree = { SECTION: [ // c2 { TEXT_LEAF: [] }, // Some text - { TEXT_LEAF: [] } // More text + { TEXT_LEAF: [] }, // More text + { PUSHBUTTON: [] } // The button we appended. ] }; testAccessibleTree(iframe.contentDocument.getElementById("c2"), tree);