search

w3c / accname

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

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

Closed accdc closed 7 months ago

accdc commented 1 year ago

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:

[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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

jnurthen commented 10 months ago

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 commented 10 months ago

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?

MelSumner commented 10 months ago

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 👍

MelSumner commented 10 months ago

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

accdc commented 10 months ago

Thanks Melanie, you're awesome!

accdc commented 10 months ago

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

MelSumner commented 8 months ago

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 commented 8 months ago

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

daniel-montalvo commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

cookiecrook commented 8 months ago

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).

accdc commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

@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 commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

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

cookiecrook commented 7 months ago

So that thread completeness isn't lost, here was the original substantive diff from Bryan: https://github.com/w3c/accname/pull/168/commits/fa737b5aacb84d7b8e50664d6b7b3f02fdcb85d5

cookiecrook commented 7 months ago

@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 commented 7 months ago

Thank you!

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

If yes I think we are good to go.