fix: Concatenate text nodes without space (#273)

eps1lon · Jun 5, 2020 · d668f72 · d668f72
1 parent b9eb5df
commit d668f72
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 14 deletions.
diff --git a/.changeset/neat-carrots-sell.md b/.changeset/neat-carrots-sell.md
@@ -0,0 +1,7 @@
+---
+"dom-accessibility-api": patch
+---
+
+fix: Concatenate text nodes without space
+
+Fixes `<h1>Hello {name}!</h1>` in `react` computing `"Hello name !"` instead of `Hello name!`.
diff --git a/README.md b/README.md
@@ -38,12 +38,12 @@ cloning. See [the test readme](/tests/README.md) for more info about the test se
 
 ### browser (Chrome)
 
-136/144 of which 5 are due to missing whitespace.
+140/144
 
 ### jsdom
 
 <details>
-<summary>report 126/159 passing of which 16 are due `::before { content }`, 14 are accessible desc, 7 are pathological </summary>
+<summary>report 124/159 passing of which 16 are due `::before { content }`, 15 are accessible desc, 4 are pathological </summary>
 
 ```bash
   web-platform-tests

diff --git a/package.json b/package.json
@@ -13,7 +13,6 @@
 		"type": "git",
 		"url": "https://github.com/eps1lon/dom-accessibility-api.git"
 	},
-	"dependencies": {},
 	"files": [
 		".browserslistrc",
 		"dist/"

diff --git a/sources/__tests__/accessible-name.js b/sources/__tests__/accessible-name.js
@@ -302,6 +302,19 @@ test.each([
 	],
 ])(`test #%#`, testMarkup);
 
+test("text nodes are not concatenated by space", () => {
+	// how React would create `<h1>Hello {name}!</h1>`
+	// which transpiles to
+	// React.createElement('h1', 'Hello ', name, '!')
+	const heading = document.createElement("h1");
+	heading.appendChild(document.createTextNode("Hello "));
+	heading.appendChild(document.createTextNode("Jill"));
+	heading.appendChild(document.createTextNode("!"));
+	renderIntoDocument(heading);
+
+	expect(heading).toHaveAccessibleName("Hello Jill!");
+});
+
 describe("prohibited naming", () => {
 	test.each([
 		["caption", "<div data-test role='caption'>table</div>"],
@@ -401,7 +414,7 @@ describe("options.getComputedStyle", () => {
 		computeAccessibleName(container.querySelector("button"));
 
 		// also mixing in a regression test for the number of calls
-		expect(window.getComputedStyle).toHaveBeenCalledTimes(4);
+		expect(window.getComputedStyle).toHaveBeenCalledTimes(3);
 	});
 
 	it("can be mocked with a fake", () => {

diff --git a/sources/accessible-name.ts b/sources/accessible-name.ts
@@ -330,11 +330,17 @@ export function computeAccessibleName(
 				recursion: true,
 			});
 			// TODO: Unclear why display affects delimiter
-			const display =
-				isElement(node) &&
-				createGetComputedStyle(node, options)(node).getPropertyValue("display");
+			// see https://github.com/w3c/accname/issues/3
+			const display = isElement(child)
+				? createGetComputedStyle(
+						child,
+						options
+						// eslint-disable-next-line no-mixed-spaces-and-tabs -- prettier bug?
+				  )(child).getPropertyValue("display")
+				: "inline";
 			const separator = display !== "inline" ? " " : "";
-			accumulatedText += `${separator}${result}`;
+			// trailing separator for wpt tests
+			accumulatedText += `${separator}${result}${separator}`;
 		});
 
 		if (isElement(node)) {

diff --git a/tests/cypress/integration/web-platform-test.js b/tests/cypress/integration/web-platform-test.js
@@ -96,10 +96,10 @@ context("wpt", () => {
 		["name_test_case_614-manual", "pass"],
 		["name_test_case_615-manual", "pass"],
 		["name_test_case_616-manual", "pass"],
-		["name_test_case_617-manual", "fail"], // whitespace, check if label children should be concetaned with a space
-		["name_test_case_618-manual", "fail"], // whitespace, see name_test_case_617-manual
-		["name_test_case_619-manual", "fail"], // whitespace, see name_test_case_617-manual
-		["name_test_case_620-manual", "fail"], // whitespace, see name_test_case_617-manual
+		["name_test_case_617-manual", "pass"],
+		["name_test_case_618-manual", "pass"],
+		["name_test_case_619-manual", "pass"],
+		["name_test_case_620-manual", "pass"],
 		["name_test_case_621-manual", "pass"],
 		["name_test_case_659-manual", "fail"], // wrong, ::before + [title] + ::after
 		["name_test_case_660-manual", "fail"], // wrong, ::before + [title] + ::after

diff --git a/tests/wpt-jsdom/to-run.yaml b/tests/wpt-jsdom/to-run.yaml
@@ -2,9 +2,9 @@ DIR: accname
 
 description_*.html: [fail, bridge not implemented]
 name_file-label-inline-block-elements-manual.html:
-  [fail, getComputedStyle display defaults not implemented]
+  [fail, whitespace issue, likely due to `display`]
 name_file-label-inline-block-styles-manual.html:
-  [fail, getComputedStyle pseudo selector not implemented]
+  [fail, missing word, unknown]
 name_test_case_552-manual.html:
   [fail, getComputedStyle pseudo selector not implemented]
 name_test_case_553-manual.html: