Fixed handling of surrounding whitespace for CSS pseudo elements, inline and block level elements, and embedded widgets. #168

accdc · 2022-09-20T21:55:07Z

This change adds logic to handle block level and inline elements where it is necessary to add a space at the beginning or end of accumulated text in order to return the most accessible names and descriptions for the accessibility tree.

Before merging, we need to confirm:

Test coverage (add link to tests)
Need updates to browsers?
- Chrome: yes/no
- Safari: yes/no
- Firefox: yes/no

[Update: assuming this stays as a note only, there will be no WPT or implementation issues... Those would come with #225. –@cookiecrook]

Preview | Diff

accdc · 2022-09-22T15:12:32Z

Note, as confirmed in Chrome Beta, this is meant to explicitly codify what is already happening in the accessibility tree.

Test pages:

Block level and inline elements: https://whatsock.github.io/w3c-alternative-text-computation/Proposed%20Name%20and%20Description%20Tests/Name%20file-label-inline-block-elements.html

Block level and inline styles: https://whatsock.github.io/w3c-alternative-text-computation/Proposed%20Name%20and%20Description%20Tests/Name%20file-label-inline-block-styles.html

cookiecrook

I don't have much confidence that we can specify an algorithm that will well work here. Perhaps better to choose a default (join w/ space for pseudos) and maybe make an explicit author opt-out?

cookiecrook · 2022-09-28T22:24:07Z

index.html

-                    <li>For <code>:after</code> pseudo elements, <a class="termref">User agents</a> MUST append <abbr title="Cascading Style Sheets">CSS</abbr>  textual content, without a space, to the textual content of the <code>current node</code>. </li>
+                    <li>For <code>:before</code> pseudo elements, <a class="termref">User agents</a> MUST prepend <abbr title="Cascading Style Sheets">CSS</abbr> textual content to the textual content of the <code>current node</code> as follows:
+                    <ul>
+                      <li>If the pseudo element includes spacing between it and the textual content of the <code>current node</code>, <a class="termref">User agents</a> MUST prepend <abbr title="Cascading Style Sheets">CSS</abbr> textual content, with a space following, to the textual content of the <code>current node</code>. </li>


Emphasis mine:

If the pseudo element includes spacing between it and the textual content of the current node

"Spacing" is ambiguous and potential clarifications may be difficult to implement or costly for performance. There's no text "spacing" between an element and its pseudos, but the visual spacing is depending on many factors that may be difficult to analyze on the fly, like padding, margin, position, text-indent, borders, position of background elements or list-style-images...

The background image or list image placement would make predictability and consistency especially difficult. It's not even element boundaries then; it'd have to analyze rasterizations of the pixel or vector data compared to background elements.

What if we said "if the pseudo element includes a space between it and the textual content of the current node..."

Maybe we could provide a code sample that demonstrates what is meant here?

I don't want folks to overthink it but it's easy to do that with an overloaded term.

I don't think a minor prose change or example will resolve this. Trying to clarify more of the ambiguity.

"spacing" or "a space" as in what?

whitespace characters?

"pseudo element includes a space [character?] between it and the textual content of the current node" (e.g. " " or other whitespace)?: TMK, there is never a character between an element and its pseudo.

space character inside the element (e.g. first child is a text node and begins with whitespace) "between" the visible element text and the ::before contents?: e.g. A prepended or appended space character inside the element <span> ←here and here→ </span>?

space character inside the element pseudo's generated content?: A content value that begins with (for ::after) or ends with (for ::before) some whitespace character? E.g. content: ' ←here and here→ '; or content: '' / ' ←here (alt value only) and here→ ';

Other non-whitespace-character space?

left padding or margin of the element that has a ::before psuedo?

right padding or margin of the element that has an ::after psuedo?

left padding or margin ::after pseudo itself?

right padding or margin ::before pseudo itself?

invisible borders (e.g. border-color: rgba(256,256,256,0);) on either the element or pseudo?

a pseudo with a fixed width whose background image does not fill the entire width (such as an image bullet in a bulleted list)

I'm certain this list is not exhaustive, but hopeful it shows the algorithm complexity and ambiguity that's being requested with the vague language. I can't approve this as written.

I think @aleventhal would agree, but I'd like his opinion, too.

@MelSumner

What if we said "if the pseudo element includes a space between it and the textual content of the current node..."

Do you mean this space?

<div> text</div> 

Or one of these spaces?

content: "text "; /* ^ */ content: "text " / "alt"; /* ^ */ content: "text" / "alt "; /* ^ */

On the call, Bryan said this should refer to whether or not the element had an inline or similar value, versus a block or similar display value. I assume the "block-like" values include inline-block, cell, and the like. I'm not sure if display:contents or other newer layout tooling values are considered "block-like" or not.

If the above is correct, I also presume that would mean if either the adjacent elements (or their pseudo when joining pseudos) had a "block-like" display then use a space joiner...

Unknown at the moment whether that is implementable, and also unknown is how far the render tree should consider "adjacent" render objects... For example, if the adjacent DOM element is display:none (or display:contents), is the expectation that the renderer check the next adjacent element or pseudo that is not display:none? What about a display:inline element with visibility:hidden?

cookiecrook · 2022-09-28T22:25:24Z

index.html

-                    <li id="step2F.iii.c">Append the <code>result</code> to the <code>accumulated text</code>. </li>
+                    <li id="step2F.iii.c">
+                    <ol>
+                      <li>If the <code>current node</code> includes spacing between adjacent elements and itself, add a space before and after the <code>result</code>.</li>


cookiecrook · 2022-09-28T22:25:31Z

index.html

+                    </li>
+                    <li>For <code>:after</code> pseudo elements, <a class="termref">User agents</a> MUST append <abbr title="Cascading Style Sheets">CSS</abbr> textual content to the textual content of the <code>current node</code> as follows:
+                    <ul>
+                      <li>If the pseudo element includes spacing between the textual content of the <code>current node</code> and itself, <a class="termref">User agents</a> MUST append <abbr title="Cascading Style Sheets">CSS</abbr> textual content, with a space preceding, to the textual content of the <code>current node</code>. </li>


cookiecrook · 2022-09-28T22:35:05Z

Changed title b/c according to the diff, I think "block and inline styled elements" is just referring to pseudo elements. If this PR was associated with an issue that included a broader scope, please link it.

accdc · 2022-09-29T00:41:27Z

FYI, Actually this PR does include changes to handle both inline and block level elements as well as CSS pseudo elements, since the only difference between has to do with how both are styled implicitly or explicitly. E.G. Styling a div as display:inline.

I don't mind if changes are needed to make this more understandable, but we do need something that says what the difference is, because at present there is nothing that says there is a difference.

Perhaps actually using the words inline or block level styling is appropriate, I'm not sure. Suggestions are welcome to make this work.

I'm not sure how you changed the title, but if you could please put that back to refer to both inline and block level styled elements I would appreciate it.

MelSumner

👍

index.html

jnurthen · 2023-11-03T23:07:50Z

This seems to touch some of the same text as #183 - need to resolve those changes in combination with that PR once it is merged.

MelSumner · 2023-11-07T21:38:14Z

ok I think I have resolved the merge conflicts. @cookiecrook do you feel like your comments are resolved or do you want to take another pass?

jnurthen

This appears to have discarded all the changes I carefully merged into main for the new idref and format for each item. Please restore all the idrefs and the visible name for each step.

MelSumner · 2023-11-08T02:35:29Z

This appears to have discarded all the changes I carefully merged into main for the new idref and format for each item. Please restore all the idrefs and the visible name for each step.

@jnurthen i do see now that some of the idrefs that weren't in the merge conflict would have been removed. I only caught the ones in the merge conflict, sorry about that! I'll update.

@accdc I'll work on this 👍

index.html

MelSumner · 2023-11-08T02:41:30Z

ok @jnurthen I went through it again; apologies for missing that top section. I think I caught them all this time.

accdc · 2023-11-08T06:23:43Z

Thanks Melanie, you're awesome!

accdc · 2023-11-09T23:31:55Z

For those assigned reviewers, we are ready to pull this in if you have no further objections.

MelSumner · 2023-12-12T14:37:52Z

In order to keep this work moving forward, I'm going to move any comments or concerns to a separate issue, so we can merge this PR.

MelSumner · 2023-12-14T18:10:00Z

@jnurthen could you look at this again before the holidays, if possible? Thank you!!

daniel-montalvo · 2024-01-02T11:56:33Z

Hi @jnurthen @cookiecrook

My understanding is that there have been changes in this PR after you both reviewed this. Are you planning on having a look at this again?

Thank you

aleventhal · 2024-01-04T18:29:16Z

Is this statement still true? “Note, as confirmed in Chrome Beta, this is meant to explicitly codify what is already happening in the accessibility tree.”

spectranaut · 2024-01-04T19:16:34Z

Discussed on: https://www.w3.org/2024/01/04-aria-minutes.html#t05

cookiecrook · 2024-01-05T03:06:03Z

Discussed on: https://www.w3.org/2024/01/04-aria-minutes.html#t05

The TLDR is: the feedback re: "space" and "spacing" hasn't been changed since my last review, but I offered to try to propose some changes in a diff or commit to the branch. I recall the ARIA spec now links to the HTML definition of whitepace, so we may be able to reuse that to resolve the problems mentioned above (comment).

cookiecrook

I took an action to try to fix this, but the only correction I can think to make is to delete most of the diff. We may need to deep dive on this one, because the entire change relies on this concept:

If the pseudo element includes spacing between it and the textual content of the current node

And that description of "spacing" is meaningless in the context of textual space as @accdc and @MelSumner alluded to on Thursday's call. There can never be textual space between an element and its pseudo.

There can be visual spacing like a margin, or interior whitespace character inside the pseudo's content string or the element's innerText but there are no characters between between an element in the DOM and a pseudo element that is not in the DOM. The layout proximity is composited but, as I understand the CSS generated content spec, there's no time when exterior textual space comes into this equation.

accdc · 2024-01-06T06:52:07Z

This seems much more complicated than it needs to be.

For normal elements, if they are styled as block level elements, a space should be added to separate them from the prior and following content. Otherwise if styled as inline elements then no space should be added.

Why can't a pseudo element follow the same logic? It too can be either a block level or inline styled element.

I don't expect this to fix everything, but if we at least can get wording to handle this correctly it will handle most cases where these conditions are met.

accdc · 2024-01-08T18:49:15Z

Just as a bit of background, the reason why all this wording about visual spacing and so is in the spec text, is because apparently we can't actually say "block level element". Basically that's all that boils down to.

So, can you think of how to say block level element without actually saying "block level element" within the spec?

I tried and clearly failed to do this in a way that makes sense.

spectranaut · 2024-01-16T19:04:27Z

@accdc @MelSumner @cookiecrook -- Bryan asked for a deep dive in order to resolve these issues, but the agenda this week is light, so I'll put this item on the agenda for this week (Jan 18th) in the hopes that we can resolve it during normal meeting time. If someone can not attend this week we will move it, but if you can attend please come prepared to discuss.

MelSumner · 2024-01-16T19:52:53Z

@spectranaut I'll make every effort to be there, I have an appt in the morning but I'll do my best. Thanks for adding it to the agenda!

cookiecrook · 2024-01-18T18:30:52Z

WG meeting resolution: @cookiecrook to remove the problematic lines from the PR in order to move AccName forward to CR. Will file a new issue about possibly revisting CSS-AAM for display values and other reasons

spectranaut · 2024-01-18T19:14:22Z

Minutes from today's meeting: https://www.w3.org/2024/01/18-aria-minutes.html#t05

cookiecrook · 2024-01-18T22:41:46Z

index.html

-                <li id="comp_embedded_control_range"><em>Range:</em> If the embedded control has role <a class="role-reference" href="#range">range</a> (e.g., a <a class="role-reference" href="#spinbutton">spinbutton</a> or <a class="role-reference" href="#slider">slider</a>):
-                  <ol>
+             <ul>
+                <li id="comp_embedded_control_textbox">If the embedded control has role <a class="role-reference" href="#textbox">textbox</a>, return its value.</li>


Why were the step names removed? These were added when we switched off numbered IDs so that people could remember a step by its unified name. This PR seems unrelated, so I'm putting these back in.

cookiecrook · 2024-01-18T22:42:45Z

index.html

-                    <li id="comp_name_from_content_pseudo_element_before">For <code>::before</code> pseudo elements, <a class="termref">User agents</a> MUST prepend <abbr title="Cascading Style Sheets">CSS</abbr> textual content, without a space, to the textual content of the <code>current node</code>. </li>
-                    <li id="comp_name_from_content_pseudo_element_after">For <code>::after</code> pseudo elements, <a class="termref">User agents</a> MUST append <abbr title="Cascading Style Sheets">CSS</abbr> textual content, without a space, to the textual content of the <code>current node</code>. </li>
-                  </ol>
+                <li id="comp_name_from_content_reset"><span id="step2F.i"><!-- Don't link to this legacy numbered ID. --></span>Set the <code>accumulated text</code> to the empty string.</li>


Same here and throughout. The step name was removed. Was this accidental?

@MelSumner Is that related to this other merge problem? #168 (comment)

@MelSumner Nm, I see this just came off a much older branch, so these were left out of a manual merge. Hold off on changing anything. I'm going to reset the branch to main and then manually merge any of Bryan's original diff that needs to come back.

cookiecrook · 2024-01-18T22:51:51Z

index.html

-                    <li>For <code>:after</code> pseudo elements, <a class="termref">User agents</a> MUST append <abbr title="Cascading Style Sheets">CSS</abbr>  textual content, without a space, to the textual content of the <code>current node</code>. </li>
+                    <li>For <code>:before</code> pseudo elements, <a class="termref">User agents</a> MUST prepend <abbr title="Cascading Style Sheets">CSS</abbr> textual content to the textual content of the <code>current node</code> as follows:
+                    <ul>
+                      <li>If the pseudo element includes spacing between it and the textual content of the <code>current node</code>, <a class="termref">User agents</a> MUST prepend <abbr title="Cascading Style Sheets">CSS</abbr> textual content, with a space following, to the textual content of the <code>current node</code>. </li>


On the call, Bryan said this should refer to whether or not the element had an inline or similar value, versus a block or similar display value. I assume the "block-like" values include inline-block, cell, and the like. I'm not sure if display:contents or other newer layout tooling values are considered "block-like" or not.

If the above is correct, I also presume that would mean if either the adjacent elements (or their pseudo when joining pseudos) had a "block-like" display then use a space joiner...

Unknown at the moment whether that is implementable, and also unknown is how far the render tree should consider "adjacent" render objects... For example, if the adjacent DOM element is display:none (or display:contents), is the expectation that the renderer check the next adjacent element or pseudo that is not display:none? What about a display:inline element with visibility:hidden?

cookiecrook · 2024-01-19T02:38:11Z

So that thread completeness isn't lost, here was the original substantive diff from Bryan: fa737b5

stale

cookiecrook · 2024-01-19T02:41:35Z

@accdc I can't re-request review since this is your PR, but I've replaced the prior diffs with my own... Please review this note and the new issue #225. Thanks.

accdc · 2024-01-20T01:50:52Z

Thank you!

@MelSumner can you confirm this looks good to you now as well?

If yes I think we are good to go.

…ine and block level elements, and embedded widgets. (#168) SHA: b05bdac Reason: push, by MelSumner Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

accdc requested review from cookiecrook, aleventhal and MelSumner September 20, 2022 21:55

cookiecrook requested changes Sep 28, 2022

View reviewed changes

cookiecrook changed the title ~~Fixed handling of surrounding whitespace for block level and inline styled elements.~~ Fixed handling of surrounding whitespace for CSS pseudo elements. Sep 28, 2022

accdc changed the title ~~Fixed handling of surrounding whitespace for CSS pseudo elements.~~ Fixed handling of surrounding whitespace for CSS pseudo elements, inline and block level elements, and embedded widgets. Sep 29, 2022

jnurthen mentioned this pull request Oct 6, 2022

fix: revert changes introduced by bb65de #172

Merged

Jym77 mentioned this pull request Nov 8, 2022

Accessible name from aria-label is not computed correctly in case of inline elements Siteimprove/alfa#1203

Closed

MelSumner approved these changes Nov 14, 2022

View reviewed changes