commit e5e837d5fccfdf3d203fbe785b89556dbaab2236
parent 6cbbb9bc81e4e55a4a00101cc25cd44686f3255b
Author: Eitan Isaacson <eitan@monotonous.org>
Date: Fri, 14 Nov 2025 04:53:26 +0000
Bug 1999436 - Don't cache relation atom if its not associated with a valid tag. r=morgan
This brings us back in to compliance with `label[for]` tags. Before this
patch we would erroneously be caching `for` attributes even though we
check the DOM for it being an HTML label with a control. This was
because we ended up caching the for attribute as-is via the `output[for]`
relation even though the element is a label and not an output.
Differential Revision: https://phabricator.services.mozilla.com/D272509
Diffstat:
3 files changed, 58 insertions(+), 59 deletions(-)
diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp
@@ -4089,6 +4089,9 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache(
for (auto const& data : kRelationTypeAtoms) {
nsTArray<uint64_t> ids;
nsStaticAtom* const relAtom = data.mAtom;
+ if (data.mValidTag && !mContent->IsHTMLElement(data.mValidTag)) {
+ continue;
+ }
Relation rel;
if (data.mType == RelationType::LABEL_FOR) {
diff --git a/accessible/tests/browser/relations/browser_relations_general.js b/accessible/tests/browser/relations/browser_relations_general.js
@@ -44,19 +44,31 @@ addAccessibleTask(
*/
addAccessibleTask(
`
- <input type="checkbox" id="dependant1">
- <input type="checkbox" id="dependant2">
+ <input type="checkbox" id="dependant">
<label id="host">label</label>`,
async function (browser, accDoc) {
- await testRelated(
- browser,
- accDoc,
- "for",
- RELATION_LABEL_FOR,
- RELATION_LABELLED_BY
- );
+ const host = findAccessibleChildByID(accDoc, "host");
+ const dependant = findAccessibleChildByID(accDoc, "dependant");
+ async function testLabel(hasLabel) {
+ await testCachedRelation(
+ host,
+ RELATION_LABEL_FOR,
+ hasLabel ? dependant : []
+ );
+ await testCachedRelation(
+ dependant,
+ RELATION_LABELLED_BY,
+ hasLabel ? host : []
+ );
+ }
+
+ await testLabel(false);
+ await invokeSetAttribute(browser, "host", "for", "dependant");
+ await testLabel(true);
+ await invokeSetAttribute(browser, "dependant", "id", "invalid");
+ await testLabel(false);
},
- { iframe: true, remoteIframe: true }
+ { chrome: true, iframe: true, remoteIframe: true }
);
/**
diff --git a/accessible/tests/browser/relations/browser_relations_general_002.js b/accessible/tests/browser/relations/browser_relations_general_002.js
@@ -89,44 +89,6 @@ addAccessibleTask(
);
/*
- * Test mutation of LABEL relations via accessible shutdown.
- */
-addAccessibleTask(
- `
- <div id="d"></div>
- <label id="l">
- <select id="s">
- `,
- async function (browser, accDoc) {
- const label = findAccessibleChildByID(accDoc, "l");
- const select = findAccessibleChildByID(accDoc, "s");
- const div = findAccessibleChildByID(accDoc, "d");
-
- await testCachedRelation(label, RELATION_LABEL_FOR, select);
- await testCachedRelation(select, RELATION_LABELLED_BY, label);
- await testCachedRelation(div, RELATION_LABELLED_BY, []);
-
- const r = waitForEvent(EVENT_REORDER, "l");
- await invokeContentTask(browser, [], () => {
- content.document.getElementById("s").remove();
- });
- await r;
- await invokeContentTask(browser, [], () => {
- const l = content.document.getElementById("l");
- l.htmlFor = "d";
- });
- await testCachedRelation(label, RELATION_LABEL_FOR, div);
- await testCachedRelation(div, RELATION_LABELLED_BY, label);
- },
- {
- chrome: false,
- iframe: true,
- remoteIframe: true,
- topLevel: true,
- }
-);
-
-/*
* Test mutation of LABEL relations via DOM ID reuse.
*/
addAccessibleTask(
@@ -191,19 +153,23 @@ addAccessibleTask(
*/
addAccessibleTask(
`
- <div id="d"></div>
+ <input id="d"></input>
<label id="l">
<select id="s">
`,
async function (browser, accDoc) {
const label = findAccessibleChildByID(accDoc, "l");
const select = findAccessibleChildByID(accDoc, "s");
- const div = findAccessibleChildByID(accDoc, "d");
+ const input = findAccessibleChildByID(accDoc, "d");
await testCachedRelation(label, RELATION_LABEL_FOR, select);
await testCachedRelation(select, RELATION_LABELLED_BY, label);
- await testCachedRelation(div, RELATION_LABELLED_BY, []);
+ await testCachedRelation(input, RELATION_LABELLED_BY, []);
await untilCacheOk(() => {
+ if (!browser.isRemoteBrowser) {
+ return true;
+ }
+
try {
// We should get an acc ID back from this, but we don't have a way of
// verifying its correctness -- it should be the ID of the select.
@@ -214,12 +180,16 @@ addAccessibleTask(
}
}, "Label for relation exists");
- const r = waitForEvent(EVENT_REORDER, "l");
+ const r = waitForEvent(EVENT_INNER_REORDER, "l");
await invokeContentTask(browser, [], () => {
content.document.getElementById("s").remove();
});
await r;
await untilCacheOk(() => {
+ if (!browser.isRemoteBrowser) {
+ return true;
+ }
+
try {
label.cache.getStringProperty("for");
} catch (e) {
@@ -234,15 +204,11 @@ addAccessibleTask(
const l = content.document.getElementById("l");
l.htmlFor = "d";
});
- await testCachedRelation(label, RELATION_LABEL_FOR, div);
- await testCachedRelation(div, RELATION_LABELLED_BY, label);
+ await testCachedRelation(label, RELATION_LABEL_FOR, input);
+ await testCachedRelation(input, RELATION_LABELLED_BY, label);
},
{
- /**
- * This functionality is broken in our LocalAcccessible implementation,
- * so we avoid running this test in chrome or when the cache is off.
- */
- chrome: false,
+ chrome: true,
iframe: true,
remoteIframe: true,
topLevel: true,
@@ -374,3 +340,21 @@ addAccessibleTask(
},
{ chrome: true, topLevel: true }
);
+
+/**
+ * Test elements that are not label by spec do not get LABEL relations
+ */
+addAccessibleTask(
+ `
+ <label id="label" for="btn">label</label>
+ <div role="button" id="btn"></div>
+ `,
+ async function testLabelOnDiv(browser, docAcc) {
+ const btn = findAccessibleChildByID(docAcc, "btn");
+
+ const label = findAccessibleChildByID(docAcc, "label");
+ await testCachedRelation(btn, RELATION_LABELLED_BY, []);
+ await testCachedRelation(label, RELATION_LABEL_FOR, []);
+ },
+ { chrome: true, topLevel: true }
+);
