Skip to content

Update auto-update-text-efbfc7.md #1959

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

Closed
wants to merge 2 commits into from

Conversation

tbostic32
Copy link
Collaborator

Updates to the auto update text rule to help readability.

  1. Updated applicability to use children text nodes instead of innerText to remove some of the conditions to get the correct test target.
  2. Updated example language given the new applicability.
  3. Removed inapplicable example

Related PR: #1916

Need for Call for Review:
This will require a 1 week Call for Review


How to Review And Approve

  • Go to the “Files changed” tab
  • Here you will have the option to leave comments on different lines.
  • Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
  • Make sure to also review the proposed Call for Review period. In case of disagreement, the longer period wins.

Modifying the applicability to be more understandable.
- **changed:** the `innerText` property of the element changes multiple times within a 10 minute time span where there is no [user interaction][]; and
- **no child changed:** the element does not have [children][child] in the [flat tree][] whose `innerText` property also changes; and
- **not alone:** the element has an [ancestor][] element in the [flat tree][] with a non-empty `innerText` property whose value is different from the `innerText` of the test target.
This rule applies to any [HTML element][] that has one or more [visible][] [text nodes][] as [children][] in the [flat tree][], where the combined text from the [text nodes][] changes multiple times within a 10 minute time span where there is no [user interaction][].
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I currently have this including the possibility of multiple text nodes, we could potentially simplify this by using a normalized (https://dom.spec.whatwg.org/#dom-node-normalize) DOM, but that might make it unnecessarily more technical as well.

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, normalising would collapse adjacent text nodes, but in something like <div>Lorem <span>Ipsum</span> Dolor</div>, the div has two text nodes (Lorem and Dolor) and they won't be collpased by normalisation (due to the span in the middle) 🤔

(also looks a bit like Text.wholeText)


<script type="text/javascript" src="/test-assets/efbfc7/script.js"></script>
</body>
```
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm a little bit confused where this inapplicable example came from and couldn't find anything in the understanding docs. I would personally fail this example, so I removed it for the moment, re-adding it would mean changing the new applicability.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth adding it to the failed examples? - It is a hard call and might be worth asking @Jym77 or @carlosapaduarte for their thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

SC 2.2.2 states (emphasises mine):

For moving, blinking, scrolling, or auto-updating information, all of the following are true:

  • Moving, blinking, scrolling: For any moving, blinking or scrolling information that (1) starts automatically, (2) lasts more than five seconds, and (3) is presented in parallel with other content, there is a mechanism for the user to pause, stop, or hide it unless the movement, blinking, or scrolling is part of an activity where it is essential; and
  • Auto-updating: For any auto-updating information that (1) starts automatically and (2) is presented in parallel with other content, there is a mechanism for the user to pause, stop, or hide it or to control the frequency of the update unless the auto-updating is part of an activity where it is essential.

This content is not "presented in parallel with other content" and therefore shouldn't fail the SC.

Removing unused definitions.
@tbostic32 tbostic32 requested a review from HelenBurge October 20, 2022 13:12

<script type="text/javascript" src="/test-assets/efbfc7/script.js"></script>
</body>
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth adding it to the failed examples? - It is a hard call and might be worth asking @Jym77 or @carlosapaduarte for their thoughts?

- **changed:** the `innerText` property of the element changes multiple times within a 10 minute time span where there is no [user interaction][]; and
- **no child changed:** the element does not have [children][child] in the [flat tree][] whose `innerText` property also changes; and
- **not alone:** the element has an [ancestor][] element in the [flat tree][] with a non-empty `innerText` property whose value is different from the `innerText` of the test target.
This rule applies to any [HTML element][] that has one or more [visible][] [text nodes][] as [children][] in the [flat tree][], where the combined text from the [text nodes][] changes multiple times within a 10 minute time span where there is no [user interaction][].
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, normalising would collapse adjacent text nodes, but in something like <div>Lorem <span>Ipsum</span> Dolor</div>, the div has two text nodes (Lorem and Dolor) and they won't be collpased by normalisation (due to the span in the middle) 🤔

(also looks a bit like Text.wholeText)

- **changed:** the `innerText` property of the element changes multiple times within a 10 minute time span where there is no [user interaction][]; and
- **no child changed:** the element does not have [children][child] in the [flat tree][] whose `innerText` property also changes; and
- **not alone:** the element has an [ancestor][] element in the [flat tree][] with a non-empty `innerText` property whose value is different from the `innerText` of the test target.
This rule applies to any [HTML element][] that has one or more [visible][] [text nodes][] as [children][] in the [flat tree][], where the combined text from the [text nodes][] changes multiple times within a 10 minute time span where there is no [user interaction][].
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been doing a bit of rule archaeology… The "innerText" formulation came after this discussion, and notably this example:

<div id="x"><b>Hello world</b></div>
<script>
setTimeout(() => document.querySelector('#x').innerHTML = '<i>Goodbye</i>', 1000)
</script>

Where the div is Applicable (and fails) in the current rule but would be Inapplicable in the new rule (it has no text content), and neither the b nor the i have text node children that changes (the i could as well be a new b, different from the existing one, with regular updates).


The actual move to use innerText seems to be this commit which was done apparently after some not archived discussion.
I have some blurry memories of the fact that even something like all our examples that use element.innerText = "foo" to update the text result in creating a new text node rather than updating the existing one. @carlosapaduarte @WilcoFiers do you remember anything else?

Then of course, the new Applicability uses "combined text" which sounds like a non-technical way to say innerText. I'd argue we'd need to define "combined text" more precisely, because in our examples where the text nodes are deleted and replaced by new ones, I'm not sure whether the "combined text from the text nodes" is changing. The innerText of the parent is changing, but we need to be unambiguous that "combined text of the text nodes" means "as seen from the parent point of view".


In any case, due to the first example in this comment, the change is a big change in Applicability, making the rule much less applicable. I'm not sure this is what we want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I appreciate the rule archaeology, that does help me understand why the applicability was formulated like this. I am going to do some additional poking to see if there is a way to restate it to make it a bit simpler. At the very least (if i can't come up with anything) I would like to add a Note that discusses the context of no child changed and not alone, such as your comment above on where the not alone came from and then for no child changed how that localizes the target to the descendant closest to the changed text.

I figured I would need to define "combined text" in more definite terms. My intention is that it is calculated as the union of the text of all the text nodes of a parent( e.g., so if a parent has 3 text nodes a, b, and c, its combined text would be the union of text from a, b, c). This was my attempt to get rid of some of the tree traversal the no child changed condition brings with it. But your first example above using <b> or <i> invalidates that approach so I will have to go back to the drawing board on that.

@@ -77,7 +73,7 @@ The [instruments][instrument] used to pass this rule (if any), must meet all lev

#### Passed Example 1

This `span` element contains text content that is automatically **changed** multiple times without user intervention and there is a button available to stop the automatic changes. The rule is not applicable to the second `p` element because it has a **child changed** (the `span` element).
This `span` element contains text content that automatically changes multiple times without user intervention and there is a button available to stop the automatic changes. The rule is not applicable to the second `p` element because its child [text node][] is unchanged.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is where we need to be clear about what "contains text content" means.
The span has text nodes that change (are replaced by other text nodes). But the actual data of the individual text nodes doesn't change (as far as I remember).

I guess we'll end up saying that "contains text content" refers to the innerText 🤔


<script type="text/javascript" src="/test-assets/efbfc7/script.js"></script>
</body>
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

SC 2.2.2 states (emphasises mine):

For moving, blinking, scrolling, or auto-updating information, all of the following are true:

  • Moving, blinking, scrolling: For any moving, blinking or scrolling information that (1) starts automatically, (2) lasts more than five seconds, and (3) is presented in parallel with other content, there is a mechanism for the user to pause, stop, or hide it unless the movement, blinking, or scrolling is part of an activity where it is essential; and
  • Auto-updating: For any auto-updating information that (1) starts automatically and (2) is presented in parallel with other content, there is a mechanism for the user to pause, stop, or hide it or to control the frequency of the update unless the auto-updating is part of an activity where it is essential.

This content is not "presented in parallel with other content" and therefore shouldn't fail the SC.

@tbostic32 tbostic32 closed this Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants