From b5e0236415e7af83e026c19fb595f87f88f114b7 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 27 Oct 2019 16:38:13 +0100 Subject: [PATCH 1/4] fix: embedded control using title --- sources/accessible-name.ts | 47 ++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/sources/accessible-name.ts b/sources/accessible-name.ts index f107b5ba..0e30a945 100644 --- a/sources/accessible-name.ts +++ b/sources/accessible-name.ts @@ -421,34 +421,37 @@ export function computeAccessibleName( } // 2C - const ariaLabel = ( - (isElement(current) && current.getAttribute("aria-label")) || - "" - ).trim(); - if (ariaLabel !== "") { - consultedNodes.add(current); - if (context.recursion && isEmbeddedControl(current)) { - throw new Error("Not implemented"); - } - return ariaLabel; - } - - // 2D - if (!hasAnyConcreteRoles(current, ["none", "presentation"])) { - const elementTextAlternative = computeElementTextAlternative(current); - if (elementTextAlternative !== null) { + if (context.recursion && isEmbeddedControl(current)) { + // skip to 2E + } else { + const ariaLabel = ( + (isElement(current) && current.getAttribute("aria-label")) || + "" + ).trim(); + if (ariaLabel !== "") { consultedNodes.add(current); - return elementTextAlternative; + return ariaLabel; } - const attributeTextAlternative = computeAttributeTextAlternative(current); - if (attributeTextAlternative !== null) { - consultedNodes.add(current); - return attributeTextAlternative; + + // 2D + if (!hasAnyConcreteRoles(current, ["none", "presentation"])) { + const elementTextAlternative = computeElementTextAlternative(current); + if (elementTextAlternative !== null) { + consultedNodes.add(current); + return elementTextAlternative; + } + const attributeTextAlternative = computeAttributeTextAlternative( + current + ); + if (attributeTextAlternative !== null) { + consultedNodes.add(current); + return attributeTextAlternative; + } } } // 2E - if (context.isReferenced || context.isEmbeddedInLabel) { + if (context.isEmbeddedInLabel) { if (hasAnyConcreteRoles(current, ["combobox", "listbox"])) { consultedNodes.add(current); const selectedOptions = querySelectedOptions(current); From 1a17a76d1b9d954fb33b6d64b4c222875c1db9e6 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 27 Oct 2019 16:52:41 +0100 Subject: [PATCH 2/4] Can't be it --- sources/__tests__/accessible-name.js | 15 ++++++++++++++- sources/accessible-name.ts | 18 ++++++++++-------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js index f9835b9a..3734e243 100644 --- a/sources/__tests__/accessible-name.js +++ b/sources/__tests__/accessible-name.js @@ -126,7 +126,7 @@ describe("to upstream", () => { ); test.each([ - [ + /* [ // TODO // weird edge case that results in an empty accessible name // Intuitevly the fist input has "foo baz" while the second one has "foo David" @@ -170,6 +170,19 @@ describe("to upstream", () => { `, "Pick contributor" + ] */ + // It seems like this is what wpt `name_heading-combobox-focusable-alternative` + // should actually test. I could not find specification for combobox falling + // back to the "value" attribute when computing the text alternative for the selected option + [ + "embedded textbox", + ` +

+ Country of origin: + +

+`, + "Country of origin: United States" ] ])(`coverage for %s`, (_, markup, expectedAccessibleName) => { return testMarkup(markup, expectedAccessibleName); diff --git a/sources/accessible-name.ts b/sources/accessible-name.ts index 0e30a945..29f50974 100644 --- a/sources/accessible-name.ts +++ b/sources/accessible-name.ts @@ -123,12 +123,14 @@ function queryChildNodes(node: Node): Node[] { } /** - * * @param {Node} node - - * @returns {boolean} - + * @returns {boolean} - As defined in step 2E of https://w3c.github.io/accname/#mapping_additional_nd_te */ -function isEmbeddedControl(node: Node): boolean { - return false; +function isControl(node: Node): boolean { + return ( + hasAnyConcreteRoles(node, ["button", "combobox", "listbox", "textbox"]) || + hasAbstractRole(node, "range") + ); } function hasAbstractRole(node: Node, role: string): node is Element { @@ -421,9 +423,9 @@ export function computeAccessibleName( } // 2C - if (context.recursion && isEmbeddedControl(current)) { - // skip to 2E - } else { + const skipToStep2E = + context.recursion && context.isEmbeddedInLabel && isControl(current); + if (!skipToStep2E) { const ariaLabel = ( (isElement(current) && current.getAttribute("aria-label")) || "" @@ -451,7 +453,7 @@ export function computeAccessibleName( } // 2E - if (context.isEmbeddedInLabel) { + if (skipToStep2E || context.isEmbeddedInLabel) { if (hasAnyConcreteRoles(current, ["combobox", "listbox"])) { consultedNodes.add(current); const selectedOptions = querySelectedOptions(current); From efcc8bd563a2b445c582d55a40ffa9f895863888 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sun, 27 Oct 2019 17:09:02 +0100 Subject: [PATCH 3/4] Fix by considering looping through IDREFs not as recursion --- sources/__tests__/accessible-name.js | 4 ++-- sources/accessible-name.ts | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js index 3734e243..8380010e 100644 --- a/sources/__tests__/accessible-name.js +++ b/sources/__tests__/accessible-name.js @@ -126,7 +126,7 @@ describe("to upstream", () => { ); test.each([ - /* [ + [ // TODO // weird edge case that results in an empty accessible name // Intuitevly the fist input has "foo baz" while the second one has "foo David" @@ -170,7 +170,7 @@ describe("to upstream", () => { `, "Pick contributor" - ] */ + ], // It seems like this is what wpt `name_heading-combobox-focusable-alternative` // should actually test. I could not find specification for combobox falling // back to the "value" attribute when computing the text alternative for the selected option diff --git a/sources/accessible-name.ts b/sources/accessible-name.ts index 29f50974..d000ba95 100644 --- a/sources/accessible-name.ts +++ b/sources/accessible-name.ts @@ -416,15 +416,17 @@ export function computeAccessibleName( computeTextAlternative(element, { isEmbeddedInLabel: context.isEmbeddedInLabel, isReferenced: true, - recursion: true + // thais isn't recursion as specified, otherwise we would skip + // `aria-label` in + // Date: Sun, 27 Oct 2019 17:21:08 +0100 Subject: [PATCH 4/4] Apparently the selected option of input-combobox is the value --- README.md | 6 +++--- sources/accessible-name.ts | 5 +++-- tests/cypress/integration/web-platform-test.js | 2 +- tests/wpt-jsdom/to-run.yaml | 2 -- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 973466fe..3ad844e8 100644 --- a/README.md +++ b/README.md @@ -28,12 +28,12 @@ cloning. See tests/README.md for more info about the test setup. ### browser (Chrome) -135/144 of which 5 are due to missing whitespace. +136/144 of which 5 are due to missing whitespace. ### jsdom
-report 125/159 passing of which 16 are due `::before { content }`, 14 are accessible desc, 8 are pathological +report 126/159 passing of which 16 are due `::before { content }`, 14 are accessible desc, 7 are pathological ```bash web-platform-tests @@ -79,7 +79,7 @@ cloning. See tests/README.md for more info about the test setup. ✓ name_from_content_of_label-manual.html ✓ name_from_content_of_labelledby_element-manual.html ✓ name_from_content_of_labelledby_elements_one_of_which_is_hidden-manual.html - ✓ [expected fail] name_heading-combobox-focusable-alternative-manual.html + ✓ name_heading-combobox-focusable-alternative-manual.html ✓ name_image-title-manual.html ✓ name_link-mixed-content-manual.html ✓ name_link-with-label-manual.html diff --git a/sources/accessible-name.ts b/sources/accessible-name.ts index d000ba95..bc1fca09 100644 --- a/sources/accessible-name.ts +++ b/sources/accessible-name.ts @@ -455,12 +455,13 @@ export function computeAccessibleName( } // 2E - if (skipToStep2E || context.isEmbeddedInLabel) { + if (skipToStep2E || context.isEmbeddedInLabel || context.isReferenced) { if (hasAnyConcreteRoles(current, ["combobox", "listbox"])) { consultedNodes.add(current); const selectedOptions = querySelectedOptions(current); if (selectedOptions.length === 0) { - return ""; + // defined per test `name_heading_combobox` + return isHTMLInputElement(current) ? current.value : ""; } return Array.from(selectedOptions) .map(selectedOption => { diff --git a/tests/cypress/integration/web-platform-test.js b/tests/cypress/integration/web-platform-test.js index 42a68bf4..f6b889ff 100644 --- a/tests/cypress/integration/web-platform-test.js +++ b/tests/cypress/integration/web-platform-test.js @@ -34,7 +34,7 @@ context("wpt", () => { "name_from_content_of_labelledby_elements_one_of_which_is_hidden-manual", "pass" ], - ["name_heading-combobox-focusable-alternative-manual", "fail"], // wrong text alternative, see mailing list + ["name_heading-combobox-focusable-alternative-manual", "pass"], ["name_image-title-manual", "pass"], ["name_link-mixed-content-manual", "pass"], ["name_link-with-label-manual", "pass"], diff --git a/tests/wpt-jsdom/to-run.yaml b/tests/wpt-jsdom/to-run.yaml index b6093761..c7aa4060 100644 --- a/tests/wpt-jsdom/to-run.yaml +++ b/tests/wpt-jsdom/to-run.yaml @@ -5,8 +5,6 @@ name_file-label-inline-block-elements-manual.html: [fail, getComputedStyle display defaults not implemented] name_file-label-inline-block-styles-manual.html: [fail, getComputedStyle pseudo selector not implemented] -name_heading-combobox-focusable-alternative-manual.html: - [fail, could not find spec for that] name_test_case_552-manual.html: [fail, getComputedStyle pseudo selector not implemented] name_test_case_553-manual.html: