From 1ffa4bb82208345b609fcbf34e20bc750e663efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 4 Jun 2026 21:30:12 +0000 Subject: [PATCH] Bug 2041502 - Make PresShell forward notifications to DocAccessible. r=layout-reviewers,jwatt The main difference is that removals get processed before other observers, like PresShell does. This prevents the options list from getting re-filled, see the stack in the bug. While at it, simplify the aria attribute notifications, they don't need to walk the whole chain, as only DocAccessible cares about it. Route them through the accessible service like the AttrElement ones. Differential Revision: https://phabricator.services.mozilla.com/D304716 --- accessible/base/nsAccessibilityService.cpp | 18 +++++++ accessible/base/nsAccessibilityService.h | 11 +++++ accessible/generic/DocAccessible.cpp | 48 ------------------ accessible/generic/DocAccessible.h | 38 ++++++++------ accessible/mac/DocAccessibleWrap.h | 8 +-- accessible/tests/crashtests/2041502.html | 20 ++++++++ accessible/tests/crashtests/crashtests.list | 1 + dom/base/MutationObservers.cpp | 16 ------ dom/base/MutationObservers.h | 7 --- dom/base/nsIMutationObserver.h | 49 +++++-------------- dom/base/nsStubMutationObserver.cpp | 14 ------ dom/html/ElementInternals.cpp | 14 ++++-- layout/base/PresShell.cpp | 45 +++++++++++++++++ .../satchel/nsFormFillController.cpp | 6 --- widget/cocoa/nsMenuGroupOwnerX.mm | 6 --- 15 files changed, 143 insertions(+), 158 deletions(-) create mode 100644 accessible/tests/crashtests/2041502.html diff --git a/accessible/base/nsAccessibilityService.cpp b/accessible/base/nsAccessibilityService.cpp index 1edb6f8faa59..6281001e000a 100644 --- a/accessible/base/nsAccessibilityService.cpp +++ b/accessible/base/nsAccessibilityService.cpp @@ -755,6 +755,24 @@ void nsAccessibilityService::NotifyAttrElementChanged( } } +void nsAccessibilityService::NotifyARIAAttributeDefaultWillChange( + mozilla::dom::Element* aElement, nsAtom* aAttribute, AttrModType aModType) { + mozilla::dom::Document* doc = aElement->OwnerDoc(); + MOZ_ASSERT(doc); + if (DocAccessible* docAcc = GetDocAccessible(doc)) { + docAcc->ARIAAttributeDefaultWillChange(aElement, aAttribute, aModType); + } +} + +void nsAccessibilityService::NotifyARIAAttributeDefaultChanged( + mozilla::dom::Element* aElement, nsAtom* aAttribute, AttrModType aModType) { + mozilla::dom::Document* doc = aElement->OwnerDoc(); + MOZ_ASSERT(doc); + if (DocAccessible* docAcc = GetDocAccessible(doc)) { + docAcc->ARIAAttributeDefaultChanged(aElement, aAttribute, aModType); + } +} + void nsAccessibilityService::AriaNotify( nsINode* aNode, const nsAString& aAnnouncement, const mozilla::dom::AriaNotificationOptions& aOptions) { diff --git a/accessible/base/nsAccessibilityService.h b/accessible/base/nsAccessibilityService.h index c10a09992208..e0a853cdba80 100644 --- a/accessible/base/nsAccessibilityService.h +++ b/accessible/base/nsAccessibilityService.h @@ -313,6 +313,17 @@ class nsAccessibilityService final : public mozilla::a11y::DocManager, */ void NotifyAttrElementChanged(mozilla::dom::Element* aElement, nsAtom* aAttr); + /** + * Notify accessibility that an ARIA attribute reflected from ElementInternals + * is about to change / has changed. See dom::ElementInternals. + */ + void NotifyARIAAttributeDefaultWillChange(mozilla::dom::Element* aElement, + nsAtom* aAttribute, + AttrModType aModType); + void NotifyARIAAttributeDefaultChanged(mozilla::dom::Element* aElement, + nsAtom* aAttribute, + AttrModType aModType); + void AriaNotify(nsINode* aNode, const nsAString& aAnnouncement, const mozilla::dom::AriaNotificationOptions& aOptions); diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index 2ce25977781a..ac1eb3d8d90e 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -151,8 +151,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(DocAccessible, LocalAccessible) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DocAccessible) - NS_INTERFACE_MAP_ENTRY(nsIDocumentObserver) - NS_INTERFACE_MAP_ENTRY(nsIMutationObserver) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_END_INHERITING(HyperTextAccessible) @@ -721,9 +719,6 @@ nsRect DocAccessible::RelativeBounds(nsIFrame** aRelativeFrame) const { // DocAccessible protected member nsresult DocAccessible::AddEventListeners() { SelectionMgr()->AddDocSelectionListener(mPresShell); - - // Add document observer. - mDocumentNode->AddObserver(this); return NS_OK; } @@ -732,10 +727,6 @@ nsresult DocAccessible::RemoveEventListeners() { // Remove listeners associated with content documents NS_ASSERTION(mDocumentNode, "No document during removal of listeners."); - if (mDocumentNode) { - mDocumentNode->RemoveObserver(this); - } - if (mScrollWatchTimer) { mScrollWatchTimer->Cancel(); mScrollWatchTimer = nullptr; @@ -839,12 +830,6 @@ std::pair DocAccessible::ComputeScrollData( return {scrollPoint, scrollRange}; } -//////////////////////////////////////////////////////////////////////////////// -// nsIDocumentObserver - -NS_IMPL_NSIDOCUMENTOBSERVER_CORE_STUB(DocAccessible) -NS_IMPL_NSIDOCUMENTOBSERVER_LOAD_STUB(DocAccessible) - // When a reflected element IDL attribute changes, we might get the following // synchronous calls: // 1. AttributeWillChange for the element. @@ -1089,11 +1074,6 @@ void DocAccessible::ARIAActiveDescendantChanged(LocalAccessible* aAccessible) { } } -void DocAccessible::ContentAppended(nsIContent* aFirstNewContent, - const ContentAppendInfo&) { - MaybeHandleChangeToHiddenNameOrDescription(aFirstNewContent); -} - void DocAccessible::ElementStateChanged(dom::Document* aDocument, dom::Element* aElement, dom::ElementState aStateMask) { @@ -1225,34 +1205,6 @@ void DocAccessible::ElementStateChanged(dom::Document* aDocument, } } -void DocAccessible::CharacterDataWillChange(nsIContent* aContent, - const CharacterDataChangeInfo&) {} - -void DocAccessible::CharacterDataChanged(nsIContent* aContent, - const CharacterDataChangeInfo&) { - MaybeHandleChangeToHiddenNameOrDescription(aContent); -} - -void DocAccessible::ContentInserted(nsIContent* aChild, - const ContentInsertInfo&) { - MaybeHandleChangeToHiddenNameOrDescription(aChild); -} - -void DocAccessible::ContentWillBeRemoved(nsIContent* aChildNode, - const ContentRemoveInfo&) { -#ifdef A11Y_LOG - if (logging::IsEnabled(logging::eTree)) { - logging::MsgBegin("TREE", "DOM content removed; doc: %p", this); - logging::Node("container node", aChildNode->GetParent()); - logging::Node("content node", aChildNode); - logging::MsgEnd(); - } -#endif - ContentRemoved(aChildNode); -} - -void DocAccessible::ParentChainChanged(nsIContent* aContent) {} - //////////////////////////////////////////////////////////////////////////////// // LocalAccessible diff --git a/accessible/generic/DocAccessible.h b/accessible/generic/DocAccessible.h index f53dea900825..5ed3d4ebd7b8 100644 --- a/accessible/generic/DocAccessible.h +++ b/accessible/generic/DocAccessible.h @@ -43,7 +43,6 @@ class TNotification; * all use this class to represent the doc they contain. */ class DocAccessible : public HyperTextAccessible, - public nsIDocumentObserver, public nsSupportsWeakReference { NS_DECL_ISUPPORTS_INHERITED NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DocAccessible, LocalAccessible) @@ -54,14 +53,11 @@ class DocAccessible : public HyperTextAccessible, public: DocAccessible(Document* aDocument, PresShell* aPresShell); - // nsIDocumentObserver - NS_DECL_NSIDOCUMENTOBSERVER - // LocalAccessible virtual void Init(); - virtual void Shutdown() override; - virtual nsIFrame* GetFrame() const override; - virtual nsINode* GetNode() const override; + void Shutdown() override; + nsIFrame* GetFrame() const override; + nsINode* GetNode() const override; Document* DocumentNode() const { return mDocumentNode; } virtual mozilla::a11y::ENameValueFlag DirectName( @@ -442,6 +438,26 @@ class DocAccessible : public HyperTextAccessible, */ uint64_t EffectiveCacheDomains() const; + /** + * For hidden subtrees, fire a name/description change event if the subtree + * is a target of aria-labelledby/describedby. + * This does nothing if it is called on a node which is not part of a hidden + * aria-labelledby/describedby target. + */ + void MaybeHandleChangeToHiddenNameOrDescription(nsIContent* aChild); + void AttributeWillChange(dom::Element* aElement, int32_t aNameSpaceID, + nsAtom* aAttribute, AttrModType aModType); + virtual void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID, + nsAtom* aAttribute, AttrModType aModType, + const nsAttrValue* aOldValue); + void ElementStateChanged(dom::Document* aDocument, dom::Element* aElement, + dom::ElementState aStateMask); + void ARIAAttributeDefaultWillChange(dom::Element* aElement, + nsAtom* aAttribute, AttrModType aModType); + + void ARIAAttributeDefaultChanged(dom::Element* aElement, nsAtom* aAttribute, + AttrModType aModType); + protected: virtual ~DocAccessible(); @@ -860,14 +876,6 @@ class DocAccessible : public HyperTextAccessible, */ void TrackMovedAccessible(LocalAccessible* aAcc); - /** - * For hidden subtrees, fire a name/description change event if the subtree - * is a target of aria-labelledby/describedby. - * This does nothing if it is called on a node which is not part of a hidden - * aria-labelledby/describedby target. - */ - void MaybeHandleChangeToHiddenNameOrDescription(nsIContent* aChild); - void MaybeHandleChangeToAriaActions(LocalAccessible* aAcc, const nsAtom* aAttribute); diff --git a/accessible/mac/DocAccessibleWrap.h b/accessible/mac/DocAccessibleWrap.h index f71d340adc0c..57245785964e 100644 --- a/accessible/mac/DocAccessibleWrap.h +++ b/accessible/mac/DocAccessibleWrap.h @@ -22,11 +22,11 @@ class DocAccessibleWrap : public DocAccessible { virtual ~DocAccessibleWrap(); - virtual void Shutdown() override; + void Shutdown() override; - virtual void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID, - nsAtom* aAttribute, AttrModType aModType, - const nsAttrValue* aOldValue) override; + void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID, + nsAtom* aAttribute, AttrModType aModType, + const nsAttrValue* aOldValue) override; void QueueNewLiveRegion(LocalAccessible* aAccessible); diff --git a/accessible/tests/crashtests/2041502.html b/accessible/tests/crashtests/2041502.html new file mode 100644 index 000000000000..e326753e15ae --- /dev/null +++ b/accessible/tests/crashtests/2041502.html @@ -0,0 +1,20 @@ + + + + + +