Skip to content

Commit

Permalink
Bug 1625864 - Fire state change event on LINKED change. r=Jamie,morgan
Browse files Browse the repository at this point in the history
Also stop recreating any accessible that has href modified.

Differential Revision: https://phabricator.services.mozilla.com/D71258
  • Loading branch information
eeejay committed Apr 24, 2020
1 parent b9c7327 commit 9db905e
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 42 deletions.
1 change: 1 addition & 0 deletions accessible/base/AccTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ enum AccType {
* Other accessible types.
*/
eApplicationType,
eHTMLLinkType,
eHTMLOptGroupType,
eImageMapType,
eMenuPopupType,
Expand Down
21 changes: 14 additions & 7 deletions accessible/base/nsAccessibilityService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,16 +285,23 @@ nsAccessibilityService::ListenersChanged(nsIArray* aEventChanges) {
change->GetCountOfEventListenerChangesAffectingAccessibility(&changeCount);
NS_ENSURE_SUCCESS(rv, rv);

for (uint32_t i = 0; i < changeCount; i++) {
if (changeCount) {
Document* ownerDoc = node->OwnerDoc();
DocAccessible* document = GetExistingDocAccessible(ownerDoc);

// Create an accessible for a inaccessible element having click event
// handler.
if (document && !document->HasAccessible(node) &&
nsCoreUtils::HasClickListener(node)) {
document->ContentInserted(node, node->GetNextSibling());
break;
if (document) {
Accessible* acc = document->GetAccessible(node);
if (!acc && nsCoreUtils::HasClickListener(node)) {
// Create an accessible for a inaccessible element having click event
// handler.
document->ContentInserted(node, node->GetNextSibling());
} else if (acc && acc->IsHTMLLink() && !acc->AsHTMLLink()->IsLinked()) {
// Notify of a LINKED state change if an HTML link gets a click
// listener but does not have an href attribute.
RefPtr<AccEvent> linkedChangeEvent =
new AccStateChangeEvent(acc, states::LINKED);
document->FireDelayedEvent(linkedChangeEvent);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions accessible/generic/Accessible.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class EmbeddedObjCollector;
class EventTree;
class HTMLImageMapAccessible;
class HTMLLIAccessible;
class HTMLLinkAccessible;
class HyperTextAccessible;
class ImageAccessible;
class KeyBinding;
Expand Down Expand Up @@ -576,6 +577,9 @@ class Accessible : public nsISupports {
bool IsHTMLListItem() const { return mType == eHTMLLiType; }
HTMLLIAccessible* AsHTMLListItem();

bool IsHTMLLink() const { return mType == eHTMLLinkType; }
HTMLLinkAccessible* AsHTMLLink();

bool IsHTMLOptGroup() const { return mType == eHTMLOptGroupType; }

bool IsHTMLTable() const { return mType == eHTMLTableType; }
Expand Down
34 changes: 20 additions & 14 deletions accessible/generic/DocAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ void DocAccessible::AttributeWillChange(dom::Element* aElement,
return;
}

if (aAttribute == nsGkAtoms::aria_disabled ||
if (aAttribute == nsGkAtoms::aria_disabled || aAttribute == nsGkAtoms::href ||
aAttribute == nsGkAtoms::disabled || aAttribute == nsGkAtoms::tabindex ||
aAttribute == nsGkAtoms::contenteditable) {
mPrevStateBits = accessible->State();
Expand Down Expand Up @@ -732,7 +732,7 @@ void DocAccessible::AttributeChanged(dom::Element* aElement,

// Fire accessible events iff there's an accessible, otherwise we consider
// the accessible state wasn't changed, i.e. its state is initial state.
AttributeChangedImpl(accessible, aNameSpaceID, aAttribute);
AttributeChangedImpl(accessible, aNameSpaceID, aAttribute, aModType);

// Update dependent IDs cache. Take care of accessible elements because no
// accessible element means either the element is not accessible at all or
Expand All @@ -748,7 +748,7 @@ void DocAccessible::AttributeChanged(dom::Element* aElement,
// DocAccessible protected member
void DocAccessible::AttributeChangedImpl(Accessible* aAccessible,
int32_t aNameSpaceID,
nsAtom* aAttribute) {
nsAtom* aAttribute, int32_t aModType) {
// Fire accessible event after short timer, because we need to wait for
// DOM attribute & resulting layout to actually change. Otherwise,
// assistive technology will retrieve the wrong state/value/selection info.
Expand Down Expand Up @@ -921,6 +921,23 @@ void DocAccessible::AttributeChangedImpl(Accessible* aAccessible,
if (aAttribute == nsGkAtoms::value) {
if (aAccessible->IsProgress())
FireDelayedEvent(nsIAccessibleEvent::EVENT_VALUE_CHANGE, aAccessible);
return;
}

if (aModType == dom::MutationEvent_Binding::REMOVAL ||
aModType == dom::MutationEvent_Binding::ADDITION) {
if (aAttribute == nsGkAtoms::href) {
if (aAccessible->IsHTMLLink() &&
!nsCoreUtils::HasClickListener(aAccessible->GetContent())) {
RefPtr<AccEvent> linkedChangeEvent =
new AccStateChangeEvent(aAccessible, states::LINKED);
FireDelayedEvent(linkedChangeEvent);
// Fire a focusable state change event if the previous state was
// different. It may be the same if there is tabindex on this link.
aAccessible->MaybeFireFocusableStateChange(
(mPrevStateBits & states::FOCUSABLE));
}
}
}
}

Expand Down Expand Up @@ -1789,17 +1806,6 @@ bool DocAccessible::UpdateAccessibleOnAttrChange(dom::Element* aElement,
return true;
}

if (aAttribute == nsGkAtoms::href) {
// Not worth the expense to ensure which namespace these are in. It doesn't
// kill use to recreate the accessible even if the attribute was used in
// the wrong namespace or an element that doesn't support it.

// Make sure the accessible is recreated asynchronously to allow the content
// to handle the attribute change.
RecreateAccessible(aElement);
return true;
}

if (aAttribute == nsGkAtoms::aria_multiselectable &&
aElement->HasAttr(kNameSpaceID_None, nsGkAtoms::role)) {
// This affects whether the accessible supports SelectAccessible.
Expand Down
3 changes: 2 additions & 1 deletion accessible/generic/DocAccessible.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,10 @@ class DocAccessible : public HyperTextAccessibleWrap,
* @param aAccessible [in] accessible the DOM attribute is changed for
* @param aNameSpaceID [in] namespace of changed attribute
* @param aAttribute [in] changed attribute
* @param aModType [in] modification type (changed/added/removed)
*/
void AttributeChangedImpl(Accessible* aAccessible, int32_t aNameSpaceID,
nsAtom* aAttribute);
nsAtom* aAttribute, int32_t aModType);

/**
* Fire accessible events when ARIA attribute is changed.
Expand Down
6 changes: 4 additions & 2 deletions accessible/html/HTMLLinkAccessible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ using namespace mozilla::a11y;

HTMLLinkAccessible::HTMLLinkAccessible(nsIContent* aContent,
DocAccessible* aDoc)
: HyperTextAccessibleWrap(aContent, aDoc) {}
: HyperTextAccessibleWrap(aContent, aDoc) {
mType = eHTMLLinkType;
}

////////////////////////////////////////////////////////////////////////////////
// nsIAccessible
Expand Down Expand Up @@ -107,7 +109,7 @@ already_AddRefed<nsIURI> HTMLLinkAccessible::AnchorURIAt(
}

////////////////////////////////////////////////////////////////////////////////
// Protected members
// HTMLLinkAccessible

bool HTMLLinkAccessible::IsLinked() const {
EventStates state = mContent->AsElement()->State();
Expand Down
14 changes: 9 additions & 5 deletions accessible/html/HTMLLinkAccessible.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,21 @@ class HTMLLinkAccessible : public HyperTextAccessibleWrap {
virtual already_AddRefed<nsIURI> AnchorURIAt(
uint32_t aAnchorIndex) const override;

protected:
virtual ~HTMLLinkAccessible() {}

enum { eAction_Jump = 0 };

/**
* Returns true if the link has href attribute.
*/
bool IsLinked() const;

protected:
virtual ~HTMLLinkAccessible() {}

enum { eAction_Jump = 0 };
};

inline HTMLLinkAccessible* Accessible::AsHTMLLink() {
return IsHTMLLink() ? static_cast<HTMLLinkAccessible*>(this) : nullptr;
}

} // namespace a11y
} // namespace mozilla

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@
getNode("scrollable2").setAttribute("disabled", "true");
await p;

p = waitForEvent(...focusableStateChange("link", false));
getNode("link").removeAttribute("href");
await p;

SimpleTest.finish();
}

Expand Down Expand Up @@ -152,5 +156,7 @@

<textarea id="textarea" rows="3" cols="30"></textarea>

<a id="link" href="#">A link</a>

</body>
</html>
20 changes: 20 additions & 0 deletions accessible/tests/mochitest/events/test_statechange.html
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,20 @@
await p;
}

async function testLinked() {
let p = waitForEvent(...stateChange(STATE_LINKED, false, false, "link1"));
getNode("link1").removeAttribute("href");
await p;

p = waitForEvent(...stateChange(STATE_LINKED, false, false, "link2"));
getNode("link2").removeAttribute("onclick");
await p;

p = waitForEvent(...stateChange(STATE_LINKED, false, true, "link3"));
getNode("link3").setAttribute("href", "http://example.com");
await p;
}

async function doTests() {
// Test opening details objects
await openNode("detailsOpen", "summaryOpen", true);
Expand Down Expand Up @@ -159,6 +173,8 @@
await echoingStateChange("text1", "aria-disabled", "disabled", null,
EXT_STATE_ENABLED, true, true);

await testLinked();

SimpleTest.finish();
}

Expand Down Expand Up @@ -245,6 +261,10 @@

<input id="text1">

<a id="link1" href="#">I am a link link</a>
<a id="link2" onclick="console.log('hi')">I am a link-ish link</a>
<a id="link3">I am a non-link link</a>

<div id="eventdump"></div>
</body>
</html>
2 changes: 1 addition & 1 deletion accessible/tests/mochitest/relations/test_update.html
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
testRelation(aLabelID, RELATION_LABEL_FOR, aElmID);
testRelation(aElmID, RELATION_LABELLED_BY, aLabelID);

this.containerNode.setAttribute('href', 'foo');
this.containerNode.setAttribute('role', 'group');
};

this.finalCheck = function recreateRelatives_finalCheck() {
Expand Down
6 changes: 0 additions & 6 deletions accessible/tests/mochitest/treeupdate/test_recreation.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@
document.getElementById("div1").setAttribute("role", "button");
await events;

msg = "Add an href to an anchor tag";
events = waitForOrderedEvents(
[[EVENT_HIDE, "anchor"], [EVENT_SHOW, "anchor"], [EVENT_REORDER, "container"]], msg);
document.getElementById("anchor").setAttribute("href", "www");
await events;

msg = "Set a listbox as multiselectable";
events = waitForOrderedEvents(
[[EVENT_HIDE, "div3"], [EVENT_SHOW, "div3"], [EVENT_REORDER, "container"]], msg);
Expand Down
12 changes: 6 additions & 6 deletions accessible/tests/mochitest/treeupdate/test_select.html
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,16 @@
}

/**
* Setting @href on option makes the accessible to recreate.
* Setting role=option on option makes the accessible recreate.
*/
function setHrefOnOption() {
function setRoleOnOption() {
this.eventSeq = [
new invokerChecker(EVENT_HIDE, "s2_o"),
new invokerChecker(EVENT_SHOW, "s2_o"),
];

this.invoke = function setHrefOnOption_setHref() {
getNode("s2_o").setAttribute("href", "1");
this.invoke = function setRoleOnOption_setRole() {
getNode("s2_o").setAttribute("role", "option");
};

this.finalCheck = function() {
Expand All @@ -106,7 +106,7 @@
};

this.getID = function removeptions_getID() {
return "setting @href on select option";
return "setting role=option on select option";
};
}

Expand All @@ -115,7 +115,7 @@

gQueue.push(new addOptions("select"));
gQueue.push(new removeOptions("select"));
gQueue.push(new setHrefOnOption());
gQueue.push(new setRoleOnOption());

gQueue.invoke(); // Will call SimpleTest.finish();
}
Expand Down

0 comments on commit 9db905e

Please sign in to comment.