Skip to content

New rule: 'Status text update has aria-live property' #1590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

ajanec01
Copy link
Collaborator

@ajanec01 ajanec01 commented Apr 27, 2021

This is the very first go at the applicability, expectation and assumption section for a new rule, 'Status text update has aria-live property'

Blocked: waiting for decision on how to handle test instructions

Comment on lines 35 to 37
This rule applies to any [HTML element][] that has a [text node][] as a [descendant][] in the [flat tree][] if:

- **changed**: the `innerText` property of the element changes; and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will create weird interactions.

<body>
  <div aria-live="assertive">
    <span id="updating">Lorem Ipsum</span>
  </div>
</body>

So, the 3 elements match the first condition (having a text node descendant). Only the span matches changed. But then the span will fail the expectation for not having an aria-live property. But I kinda guess this is OK.


## Expectation

Each test target has an implicit or explicit `aria-live` property value that is [ASCII lowercase][] match for string "assertive", "polite", or "off".
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, off being the default value means that the attribute value of aria-live is always going to be one of these three (since that is the only possible values).

Copy link
Collaborator Author

@ajanec01 ajanec01 May 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not necessarily, the implicit value of role="alert" is "assertive" which I consider to be the default value. The idea here was to make sure that aria-live property has one of the allowed values, as you mentioned, but also to make sure that cases, where updates are turned off by an instrument, are not failing the rule. That is, when there is a button that toggles between "off" and "polite" depending on the context, just like the ARIA 1.2 carousel example.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But aria-live only allows values assertive, polite or off. So it is not possible to fail this expectation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, of course, the word property is already implying that only one of the allowed values can be used as the attribute value. I think this expectation is ok. Do you recommend adding something about the default value or shortening it only to "Each test target has an implicit or explicit aria-live property"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Maybe I just need to see a Failing Example for the rule. I'm currently unable to build one…

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard my comment about the off being default! I think that we need to ditch the off value.
So, in the following example:

<div role="status" aria-live="politeness">Status message 1 of 10</div>

When checking UIA in Inspect using Chrome, there is no LiveSettingProperty. As soon as aria-live=politeness" is removed, the LiveSettingProperty appears and it has the value of Polite, which is correct.

When doing the same in Edge, the LiveSettingProperty has value Off right from the start.

I think that removing off from the expectation will provide better results for the rule.

@Jym77
Copy link
Collaborator

Jym77 commented May 20, 2021

I think it makes sense. Some comments on the structure (there are also some nitpicky typos and similar but let's leave them for later).

  • The other rules using "change in content" are effectively triggering the even causing the changes, not the "result" of the change. Especially, the definition of "change" doesn't really tell which element or text node is changing (it's about rendered pixels), which makes this connection difficult 😞
  • This gets hairier because you want (in the examples) both cases where the content (inner text) changes and where the element is created from the void.
  • The first expectation is a bit weird in trying to look at other test targets. Essentially, at expectation level only one target is known (well, not exactly, but that the idea).

I think we can also focus on accessible changes (and ignore the visible ones). That is, if some text appears but as part of a aria-hidden element, the lack of aria-live is not an issue. Since the existing definition of changes already has an item for changes in the accessibility tree, that should be doable.

For example, we could target either the node that changes or appears, or the parent of the node that disappears, and requires that it has an ancestor with the correct aria-live.

@ajanec01
Copy link
Collaborator Author

ajanec01 commented May 20, 2021

Good stuff. I was not sure about the change in content definition but it seemed to have fitted in without writing another definition.
To summarize, your recommendation is to:

  • Split the rule into two rules, one that is testing innerText changes and another rule to test elements created from the void?
  • Make sure that the test target and its descendants are included in the accessibility tree?

As for your comment about the 1st expectation, I was trying to take into consideration cases where the text changes but on its own does not provide enough context. In those situations, it's better to add a visually hidden text that has aria-live. I'm not sure how else to describe it. There is certainly a definition needed there.

@Jym77
Copy link
Collaborator

Jym77 commented May 20, 2021

To summarize, your recommendation is to:

  • Split the rule into two rules, one that is testing innerText changes and another rule to test elements created from the void?
  • Make sure that the test target and its descendants are included in the accessibility tree?

I think most of the rule can switch to the accessibility tree since aria-live only makes sense there.
I'm not sure splitting the rule is needed, but the rule could trigger nodes in the accessibility that appear, disappear, or are modified (or maybe events that causes these change, to stay closer to the other "change" rules). That can maybe be done in a single rule.
It's also easier to trigger the events than the content that appear because the content is not always here, so it's unclear what it means as a test target when it's not here… (that's discussions with had on the other changes rules).

name: 'Status text update has `aria-live` property'
rule_type: atomic
description: |
This rule checks that any text update that meets the definition of a status message has either "polite" or "assertive" `aria-live` property.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This rule checks that any text update that meets the definition of a status message has either "polite" or "assertive" `aria-live` property.
This rule checks that any status message update has either "polite" or "assertive" `aria-live` property.


## Expectation 1

There exists at least one test target with an implicit or explicit `aria-live` value of "assertive" or "polite" for [event originated changes in the content][event originated change in the content] that communicate the same message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't quite follow the rules format. An expectation must be true/false for each test target. This makes it seem like the expectation is true only once for whatever context it is in. Presumably per HTML page? But then what about EPUB, or pages with iframes?


## Assumptions

The text changes meet the definition of [status message][]. If this is not the case, success criterion [4.1.3 Status Messages][success criterion 4.1.3 status messages] may be satisfied even if this rule failed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels rather broad. I would argue that most text changes are not status messages. I am concerned this rule is going to create more false positives than it will actual issues.

This rule applies to any [HTML element][] that has a [text node][] as a [descendant][] in the [flat tree][] if all of the following are true:

- **available**: the element is [included in the accessibility tree][]; and
- **change**: the element or its [descendant][] [text node][] value is an [event originated change in the content][].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably you're selecting all descendants because one of them will need to have aria-live. But why not select the text instead, and check that it has an ancestor? It addressed the problem I raise on expectation 1.


#### Passed Example 1

The `p` element that appears after activating submit button with no value for the first name field has `role="alert"`. `role="alert"` has an implicit `aria-live` value of "assertive".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure test cases like this can work. Up until now, we've only ever used test cases that can be used in their initial loaded stage. If we want tests like this, we're going to need to figure out some way to progamatically describe how to get the page into the state it needs to be tested in.

We should probably have a proposal on how to do this, before we can move forward with this. One way would be to use a script go get the page into the correct state and fire a custom event that implementors can listen for. That has some limitations, since it can't use state that an not be set with JS, such as actually setting :hover and :focus states. Another solution could be to include some Gherkin with the text case that tools will need to implement and run.


#### Passed Example 2

This `p` element with `role="status"` has an implicit `aria-live` value of "polite".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This `p` element with `role="status"` has an implicit `aria-live` value of "polite".
This `p` element with `role="status"` has an implicit `aria-live` value of "polite".
**[Test instructions][]**:
- Start the test when the window is loaded
- Enter "technology" into the textbox
- Click the button

@carlosapaduarte carlosapaduarte added the Blocked Blocked by another PR/Issue label Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Blocked by another PR/Issue Discussion Rule idea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants