search

w3c / accname

Accessible Name and Description Computation
https://w3c.github.io/accname/
60 stars 23 forks source link

AccName: reconsider appending space between inline text nodes #193

Open cookiecrook opened 1 year ago

cookiecrook commented 1 year ago

In review for WPT /accname/name/comp_text_node.html

<h2 class="ex" data-expectedlabel="heading label" data-testname="heading with text/comment/text nodes" id="labelTest1">
  heading<!-- with non-text node splitting concatenated text nodes -->label<!-- [sic] no extra spaces around first comment -->
</h2>

WebKit results in PASS: expected "heading label" and got "heading label" Chrome results in FAIL: expected "heading label" but got "headinglabel"

Discrepancy is that the AccName comp uses the term node, and these are two text nodes joined by a comment node and no additional whitespace.

Even if WebKit is to spec here, Chromium's implementation allows authors to choose whether they want the space, while WebKit always joins on a space, according to the spec. Worth revisiting?

Skipping this test in WPT for now.

cookiecrook commented 9 months ago

Related to #205. I'm going to check in the WPT test using the Chrome behavior, since that's what the editors have indicated should happen, even though it's not in the spec yet.

accdc commented 9 months ago

Personally I would expect "headinglabel" to be correct, because a comment is not actually renderable as anything and is actually supposed to be entirely ignored within the rendered page structure, so there would be no value in including this for any type of additional processing with AccName. That would be like processing <input type="hidden"> which wouldn't make any sense either.

If this makes sense to others we can clarify it within the spec text.

cookiecrook commented 9 months ago

Okay. I've updated the tests to support your stated change here, so WebKit is currently failing the tests (to be linked once WPT is merged), despite it being the only implementation to match the spec. https://github.com/web-platform-tests/wpt/pull/42407

Please update the spec soon to match this new expectation.