Skip to content

Commit f2b94d8

Browse files
committed
Bug 1870783 part 4: Fire state change events on all popover invokers. r=eeejay
A popover can have multiple invoker buttons. Previously, we only fired a state change event on the button which was invoked. This meant that the cached expanded/collapsed state of any other invokers was stale. To facilitate this: 1. Add popovertarget to DocAccessible's kRelationAttrs so that we track reverse relationships. This will also later be used for exposing relations. 2. When an Accessible is shown or hidden, if it is a popover, use RelatedAccIterator to get all the invokers and fire appropriate state change events on each of them. 3. This also means we can get rid of nsAccessibilityService::PopovertargetMaybeChanged, since this explicit notification from DOM is no longer useful. 4. Add popovertarget to LocalAccessible::AttributeChangesState so that we fire an event for the expandable/expanded/collapsed state change if appropriate when that attribute is changed. Differential Revision: https://phabricator.services.mozilla.com/D199842
1 parent adc647d commit f2b94d8

7 files changed

+68
-55
lines changed

accessible/base/nsAccessibilityService.cpp

-13
Original file line numberDiff line numberDiff line change
@@ -716,19 +716,6 @@ void nsAccessibilityService::TableLayoutGuessMaybeChanged(
716716
}
717717
}
718718

719-
void nsAccessibilityService::PopovertargetMaybeChanged(PresShell* aPresShell,
720-
nsIContent* aContent) {
721-
DocAccessible* document = GetDocAccessible(aPresShell);
722-
if (!document) {
723-
return;
724-
}
725-
if (LocalAccessible* acc = document->GetAccessible(aContent)) {
726-
RefPtr<AccEvent> expandedChangeEvent =
727-
new AccStateChangeEvent(acc, states::EXPANDED);
728-
document->FireDelayedEvent(expandedChangeEvent);
729-
}
730-
}
731-
732719
void nsAccessibilityService::ComboboxOptionMaybeChanged(
733720
PresShell* aPresShell, nsIContent* aMutatingNode) {
734721
DocAccessible* document = GetDocAccessible(aPresShell);

accessible/base/nsAccessibilityService.h

-6
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,6 @@ class nsAccessibilityService final : public mozilla::a11y::DocManager,
180180
void TableLayoutGuessMaybeChanged(mozilla::PresShell* aPresShell,
181181
nsIContent* aContent);
182182

183-
/**
184-
* Notifies when an element's popovertarget shows/hides.
185-
*/
186-
void PopovertargetMaybeChanged(mozilla::PresShell* aPresShell,
187-
nsIContent* aContent);
188-
189183
/**
190184
* Notifies when a combobox <option> text or label changes.
191185
*/

accessible/generic/DocAccessible.cpp

+24-9
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,12 @@ using namespace mozilla::a11y;
5858
////////////////////////////////////////////////////////////////////////////////
5959
// Static member initialization
6060

61-
static nsStaticAtom* const kRelationAttrs[] = {nsGkAtoms::aria_labelledby,
62-
nsGkAtoms::aria_describedby,
63-
nsGkAtoms::aria_details,
64-
nsGkAtoms::aria_owns,
65-
nsGkAtoms::aria_controls,
66-
nsGkAtoms::aria_flowto,
67-
nsGkAtoms::aria_errormessage,
68-
nsGkAtoms::_for,
69-
nsGkAtoms::control};
61+
static nsStaticAtom* const kRelationAttrs[] = {
62+
nsGkAtoms::aria_labelledby, nsGkAtoms::aria_describedby,
63+
nsGkAtoms::aria_details, nsGkAtoms::aria_owns,
64+
nsGkAtoms::aria_controls, nsGkAtoms::aria_flowto,
65+
nsGkAtoms::aria_errormessage, nsGkAtoms::_for,
66+
nsGkAtoms::control, nsGkAtoms::popovertarget};
7067

7168
static const uint32_t kRelationAttrsLen = ArrayLength(kRelationAttrs);
7269

@@ -2006,6 +2003,22 @@ bool InsertIterator::Next() {
20062003
return false;
20072004
}
20082005

2006+
void DocAccessible::MaybeFireEventsForChangedPopover(LocalAccessible *aAcc) {
2007+
dom::Element* el = aAcc->Elm();
2008+
if (!el || !el->IsHTMLElement() || !el->HasAttr(nsGkAtoms::popover)) {
2009+
return; // Not a popover.
2010+
}
2011+
// A popover has just been inserted into or removed from the a11y tree, which
2012+
// means it just appeared or disappeared. Fire expanded state changes on its
2013+
// invokers.
2014+
RelatedAccIterator invokers(mDoc, el, nsGkAtoms::popovertarget);
2015+
while (Accessible* invoker = invokers.Next()) {
2016+
RefPtr<AccEvent> expandedChangeEvent =
2017+
new AccStateChangeEvent(invoker->AsLocal(), states::EXPANDED);
2018+
FireDelayedEvent(expandedChangeEvent);
2019+
}
2020+
}
2021+
20092022
void DocAccessible::ProcessContentInserted(
20102023
LocalAccessible* aContainer, const nsTArray<nsCOMPtr<nsIContent>>* aNodes) {
20112024
// Process insertions if the container accessible is still in tree.
@@ -2065,6 +2078,7 @@ void DocAccessible::ProcessContentInserted(
20652078
CreateSubtree(iter.Child());
20662079
mt.AfterInsertion(iter.Child());
20672080
inserted = true;
2081+
MaybeFireEventsForChangedPopover(iter.Child());
20682082
continue;
20692083
}
20702084

@@ -2603,6 +2617,7 @@ void DocAccessible::CacheChildrenInSubtree(LocalAccessible* aRoot,
26032617
}
26042618

26052619
void DocAccessible::UncacheChildrenInSubtree(LocalAccessible* aRoot) {
2620+
MaybeFireEventsForChangedPopover(aRoot);
26062621
aRoot->mStateFlags |= eIsNotInDocument;
26072622
RemoveDependentIDsFor(aRoot);
26082623

accessible/generic/DocAccessible.h

+2
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,8 @@ class DocAccessible : public HyperTextAccessible,
779779
*/
780780
void MaybeHandleChangeToHiddenNameOrDescription(nsIContent* aChild);
781781

782+
void MaybeFireEventsForChangedPopover(LocalAccessible* aAcc);
783+
782784
PresShell* mPresShell;
783785

784786
// Exclusively owned by IPDL so don't manually delete it!

accessible/generic/LocalAccessible.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,8 @@ bool LocalAccessible::AttributeChangesState(nsAtom* aAttribute) {
13041304
aAttribute == nsGkAtoms::aria_busy ||
13051305
aAttribute == nsGkAtoms::aria_multiline ||
13061306
aAttribute == nsGkAtoms::aria_multiselectable ||
1307-
aAttribute == nsGkAtoms::contenteditable;
1307+
aAttribute == nsGkAtoms::contenteditable ||
1308+
aAttribute == nsGkAtoms::popovertarget;
13081309
}
13091310

13101311
void LocalAccessible::DOMAttributeChanged(int32_t aNameSpaceID,

accessible/tests/browser/e10s/browser_caching_states.js

+40-10
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ addAccessibleTask(
490490
`
491491
<button id="show-popover-btn" popovertarget="mypopover" popovertargetaction="show">Show popover</button>
492492
<button id="hide-popover-btn" popovertarget="mypopover" popovertargetaction="hide">Hide popover</button>
493+
<button id="toggle">toggle</button>
493494
<div id="mypopover" popover>
494495
Popover content
495496
<button id="hide-inside" popovertarget="mypopover" popovertargetaction="hide">Hide inside popover</button>
@@ -500,23 +501,52 @@ addAccessibleTask(
500501
const hide = findAccessibleChildByID(docAcc, "hide-popover-btn");
501502
testStates(show, STATE_COLLAPSED, 0);
502503
testStates(hide, STATE_COLLAPSED, 0);
504+
const toggle = findAccessibleChildByID(docAcc, "toggle");
505+
testStates(
506+
toggle,
507+
0,
508+
0,
509+
STATE_EXPANDED | STATE_COLLAPSED,
510+
EXT_STATE_EXPANDABLE
511+
);
503512

513+
info("Setting toggle's popovertarget");
514+
let stateChanged = waitForStateChange(
515+
toggle,
516+
EXT_STATE_EXPANDABLE,
517+
true,
518+
true
519+
);
520+
await invokeContentTask(browser, [], () => {
521+
content.document
522+
.getElementById("toggle")
523+
.setAttribute("popovertarget", "mypopover");
524+
});
525+
await stateChanged;
526+
527+
// Changes to the popover should fire events on all invokers.
528+
const changeEvents = [
529+
[EVENT_STATE_CHANGE, show],
530+
[EVENT_STATE_CHANGE, hide],
531+
[EVENT_STATE_CHANGE, toggle],
532+
];
504533
info("Expanding popover");
505-
let onShowing = waitForEvent(EVENT_STATE_CHANGE, show);
534+
let onShowing = waitForEvents(changeEvents);
506535
await show.doAction(0);
507-
let showingEvent = await onShowing;
508-
testStates(showingEvent.accessible, STATE_EXPANDED, 0);
509-
const hideInside = findAccessibleChildByID(
510-
showingEvent.accessible,
511-
"hide-inside"
512-
);
536+
await onShowing;
537+
testStates(show, STATE_EXPANDED, 0);
538+
testStates(hide, STATE_EXPANDED, 0);
539+
testStates(toggle, STATE_EXPANDED, 0);
540+
const hideInside = findAccessibleChildByID(show, "hide-inside");
513541
testStates(hideInside, 0, 0, STATE_EXPANDED | STATE_COLLAPSED, 0);
514542

515543
info("Collapsing popover");
516-
let onHiding = waitForEvent(EVENT_STATE_CHANGE, hide);
544+
let onHiding = waitForEvents(changeEvents);
517545
await hide.doAction(0);
518-
let hidingEvent = await onHiding;
519-
testStates(hidingEvent.accessible, STATE_COLLAPSED, 0);
546+
await onHiding;
547+
testStates(hide, STATE_COLLAPSED, 0);
548+
testStates(show, STATE_COLLAPSED, 0);
549+
testStates(toggle, STATE_COLLAPSED, 0);
520550
},
521551
{ chrome: true, topLevel: true, remoteIframe: true }
522552
);

dom/html/nsGenericHTMLElement.cpp

-16
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,6 @@
9393
#include "mozilla/dom/ElementBinding.h"
9494
#include "mozilla/dom/ElementInternals.h"
9595

96-
#ifdef ACCESSIBILITY
97-
# include "nsAccessibilityService.h"
98-
#endif
99-
10096
using namespace mozilla;
10197
using namespace mozilla::dom;
10298

@@ -2865,18 +2861,6 @@ void nsGenericHTMLFormControlElementWithState::HandlePopoverTargetAction() {
28652861
} else if (shouldShow) {
28662862
target->ShowPopoverInternal(this, IgnoreErrors());
28672863
}
2868-
#ifdef ACCESSIBILITY
2869-
// Notify the accessibility service about the change.
2870-
if (shouldHide || shouldShow) {
2871-
if (RefPtr<Document> doc = GetComposedDoc()) {
2872-
if (PresShell* presShell = doc->GetPresShell()) {
2873-
if (nsAccessibilityService* accService = GetAccService()) {
2874-
accService->PopovertargetMaybeChanged(presShell, this);
2875-
}
2876-
}
2877-
}
2878-
}
2879-
#endif
28802864
}
28812865

28822866
void nsGenericHTMLFormControlElementWithState::GetInvokeAction(

0 commit comments

Comments
 (0)