New Rule: Element language is programmatically determinable #1027

patrickhlauke · 2019-11-21T10:25:42Z

This rule is primarily aimed at catching situations where 3.1.1 Language of Page fails, which also leads to automatically failing 3.1.2 (since if the language for the overall page cannot be programmatically
determined, neither can the language for any text nodes in the page itself unless there's been some explicit language definition further down in the DOM tree)

Closes issue(s):

"Language of parts" and failure when ancestors/document itself lack language definition #1009

Need for Final Call:

This will require a 2 weeks Final Call

Pull Request Etiquette

When creating PR:

Make sure you requesting to pull a branch (right side) to the develop branch (left side).

After creating PR:

Add yourself (and co-authors) as "Assignees" for PR.
Add label to indicate if it's a Rule, Definition or Chore.
Close the issue that the PR resolves (and make sure the issue is referenced in the top of this comment)
Optionally request feedback from anyone in particular by assigning them as "Reviewers".

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 Final Call period. In case of disagreement, the longer period wins.

This rule is primarily aimed at catching situations where 3.1.1 Language of Page fails, which also leads to automatically failing 3.1.2 (since if the language for the overall page cannot be programmatically determined, neither can the language for any text nodes in the page itself unless there's been some explicit language definition further down in the DOM tree) Closes #1009

CLAassistant · 2019-11-21T10:26:07Z

All committers have signed the CLA.

patrickhlauke · 2019-11-21T10:35:52Z

(note that I made this contribution from my own fork, as I don't think I can just clone and add a new branch directly here? for that reason, I'm also unable to add a label/assign myself)

patrickhlauke · 2019-11-21T10:36:33Z

I'll also add that this is my first stab at doing an ACT rule, so I expect that I made some fundamental logic/form errors...

Jym77

This looks like a good start 😄 Most of my comments are on global style issues and not on the logic of the rule…
Additional points:

many links are missing. Notably, "included in the accessibility tree", "flat tree", all the Success Criteria, … Please add them all over the place. You can use "footnote style" reference of markdown for links that are used several time.
Add the Understanding document to Background. Also add CSS scoping with the WiP mention given that you reference it (check what is done in other rules).
Given that you work on the flat tree (which makes sense), you need to add example with shadow tree. Given the nature of the rule, probably something where the shadow tree + flattening is "moving" a lang attribute. Maybe something with a slot where the slotted element has no lang ancestor in the light tree, but shadow tree manipulation put it under an element with a lang (or the other way around).
Maybe having an example with an iframe to show that the rule stops at document boundaries.
There can also be a failed example where one element is OK but another one is not. This illustrates that the SC fails if there is at least one failed outcome.

Good job!

_rules/element-lang-programmatically-determinable-7ed469.md

Jym77 · 2019-11-21T11:22:03Z

_rules/element-lang-programmatically-determinable-7ed469.md

+rule_type: atomic
+description: |
+  This rule checks that the language of an element in the page body can be programmatically determined.
+accessibility_requirements: # Remove whatever is not applicable


I think the rule is also mapping to 3.1.1: if an element has no ancestor with a valid lang, then the document element has no valid lang.

i understood accessibility_requirements as the list of (in this case) WCAG SCs that this is used to test conformance to. as this rule is scoped to descendants of body, it cannot be used to test 3.1.1 language of the page itself, as that needs to (in HTML) be defined for the html element?

I agree that the rule is not designed to test SC 3.1.1, but it still does test a bit of it indirectly…
As always, we focus on the fail-on-fail relationship. A failure of this rule indicates a failure of SC 3.1.1 (in addition to 3.1.2). A Pass on this rule does not indicate a Pass of 3.1.1, but neither of 3.1.2… That's how it is for most rules.

Then, it is true also that a failure of this rule implies a failure of HTML page has lang attribute, and that is why it fails 3.1.1. That's also OK. This rule checks more.

wondering now if another failure purely for this rule (which i've not added here, but can if it's deemed an actual failure) would be something like

<html lang="en"> <body> <div lang="foo"> <p>Content</p> </div> </body> </html>

where the p fails because its ancestor div has an invalid language attribute, but the page itself does pass 3.1.1...and so a failure of this rule does not indicate a failure of 3.1.1

The rule currently does not fail that because "any of its [the p] ancestor" has a valid lang tag.
I agree that this is a failure of 3.1.2 but not of 3.1.1

I see 2 possibilities:
=> Change the rule to "its closest ancestor with a lang attribute" or something similar. This makes the rule more tricky. In this case, it would indeed not map to 3.1.1
=> Leave the rule as is. This keeps the rule simpler and more focused. This case is already handled by Element within body has valid lang attribute, so there is no real need to enforce it in this rule. The main "problem" with this solution is that Passed Examples must pass the SC, so we cannot use this as a Passed Example to illustrate the limitation of the rule. This is not really a big deal…

Given that this case is already handle by anther rule, I'd rather keep this rule simple.

_rules/element-lang-programmatically-determinable-7ed469.md

patrickhlauke · 2019-11-21T11:44:57Z

Just on this point

There can also be a failed example where one element is OK but another one is not. This illustrates that the SC fails if there is at least one failed outcome.

doesn't the rule get applied to every single element separately, in isolation? Don't quite understand therefore why there should be an example with two separate elements, as they'd be tested individually?

Jym77 · 2019-11-21T11:55:37Z

Yes, the rule is applied to every element.

<body>
  <p lang="en" id="ok">Hello</p>
  <p id="ko">World</p>
</body>

the ok element is passing the rule. The ko element is failing the rule. As a result of a single failure of the rule, the SC also fails. Having a Failed Example like that remind of that "single fail is enough" relationship to the SC.
Not a very strong point for adding the example, I admit 😄 And I won't push it further if you or others think it's not necessarily.

Jym77 · 2019-11-21T12:17:24Z

Also, I forgot to mention that you need to add yourself as "contributor" in package.json. You can either do it as part of this PR or open a separate one. Given the usual delay we have for finishing rule PRs, it might be worth making a quick separate PR for this instead…

- add relevant links and background references - remove redundant `style="display:none"` inapplicable example - add failed example with invalid language subtag - add failed example which includes another passing element - remove note about examples also failing 3.1.1 - more specifically scope applicability to elements that have text nodes as child elements - make the description for examples slightly less robotic sounding "Language" > "The language"

patrickhlauke · 2019-11-21T12:56:35Z

many links are missing. Notably, "included in the accessibility tree", "flat tree", all the Success Criteria, … Please add them all over the place. You can use "footnote style" reference of markdown for links that are used several time.

Add the Understanding document to Background. Also add CSS scoping with the WiP mention given that you reference it (check what is done in other rules).

Given that you work on the flat tree (which makes sense), you need to add example with shadow tree. Given the nature of the rule, probably something where the shadow tree + flattening is "moving" a lang attribute. Maybe something with a slot where the slotted element has no lang ancestor in the light tree, but shadow tree manipulation put it under an element with a lang (or the other way around).

Maybe having an example with an iframe to show that the rule stops at document boundaries.

There can also be a failed example where one element is OK but another one is not. This illustrates that the SC fails if there is at least one failed outcome.

Done 1, 2 and 5. Have to admit that for 3 I have no idea what that even means, and 4 not quite sure how that would be shown/expressed in a code sample. Any hints for those two would be appreciated.

Jym77 · 2019-11-21T13:35:08Z

Given that you work on the flat tree (which makes sense), you need to add example with shadow tree. Given the nature of the rule, probably something where the shadow tree + flattening is "moving" a lang attribute. Maybe something with a slot where the slotted element has no lang ancestor in the light tree, but shadow tree manipulation put it under an element with a lang (or the other way around).

Maybe having an example with an iframe to show that the rule stops at document boundaries.

Done 1, 2 and 5. Have to admit that for 3 I have no idea what that even means, and 4 not quite sure how that would be shown/expressed in a code sample. Any hints for those two would be appreciated.

From the top of my head…

<html>
<body>
  <div lang="en">Hello
  <iframe srcdoc="<body><div>World</div></body>" />
  </div>
</body>
</html>

The div inside the iframe has no lang because in your expectation the "ancestor" is restricted in the same document. This the rule fails on it.
It's been discussed whether lang should leak inside iframe or not (it is a leak of information to an external resource). I'm not sure what the actual conclusion is 😕 but it's worth illustrating how the rule work.

For the shadow tree, … hmmm…

<html>
<body>
      <div id="host">
        <span>Hello world</span>
      </div>
      
      <script>
        const host = document.getElementById('host')
        const shadowRoot = host.attachShadow({ mode: 'open' })
      
        shadowRoot.innerHTML =
          '<div lang="en"><slot></slot></div>'
      </script>
  </body><html>

In the flat tree, the span is slotted inside the shadow tree and thus has the div lang="en" as parent. So this passes the rule even if it looks like the span has no language…
(the substitution can be made a bit more explicit by naming the slot if you want)

If you use instead shadowRoot.innerHTML='<div></div>', then there is no slot and the span disappears from the flat tree (and is not rendered), hence the rule is inapplicable. But it is less stricking (a simple host.innerHTML='<div></div>' would have the same effect).

Jym77 · 2019-11-22T08:44:42Z

_rules/element-lang-programmatically-determinable-7ed469.md

+rule_type: atomic
+description: |
+  This rule checks that the language of an element in the page body can be programmatically determined.
+accessibility_requirements: # Remove whatever is not applicable


The rule currently does not fail that because "any of its [the p] ancestor" has a valid lang tag.
I agree that this is a failure of 3.1.2 but not of 3.1.1

I see 2 possibilities:
=> Change the rule to "its closest ancestor with a lang attribute" or something similar. This makes the rule more tricky. In this case, it would indeed not map to 3.1.1
=> Leave the rule as is. This keeps the rule simpler and more focused. This case is already handled by Element within body has valid lang attribute, so there is no real need to enforce it in this rule. The main "problem" with this solution is that Passed Examples must pass the SC, so we cannot use this as a Passed Example to illustrate the limitation of the rule. This is not really a big deal…

Given that this case is already handle by anther rule, I'd rather keep this rule simple.

Jym77 · 2019-11-22T08:53:50Z

_rules/element-lang-programmatically-determinable-7ed469.md

+#### Passed Example 2
+
+The language of the `p` element is inherited by the `lang` attribute on the `div` parent element. Note that this example fails 3.1.1 Language of Page (Level: A) as the `html` root element lacks a `lang` attribute.
+
+```html
+<html>
+	<body>
+		<div lang="en">
+			<p>Content</p>
+		</div>
+	</body>
+</html>
+```
+
+#### Passed Example 3
+
+The language of the `p` element is inherited by the `lang` attribute on the `div` ancestor element. Note that this example fails 3.1.1 Language of Page (Level: A) as the `html` root element lacks a `lang` attribute.
+
+```html
+<html>
+	<body>
+		<div lang="en">
+			<div>
+				<p>Content</p>
+			</div>
+		</div>
+	</body>
+</html>
+```


These two examples are actually pretty much the same (the lang attribute is on an ancestor which is not the html element). You can remove one of them.

OTOH, I think it would be good to add an example to illustrate the "child text node" part of the rule.

<body> <div> <p lang="en">Hello</p> </div> </body>

The p is good. The div is inapplicable. So this passes the rule.

Change the rule to "its closest ancestor with a lang attribute" or something similar

I think I'm actually tending towards this one, as the closest ancestor is indeed what plays the major part in the essence of this rule, as it relates more closely in how the language of the specific element itself is calculated/programmatically determined.

And mainly, because I do think there's value in making sure that it's clear that there can be failures of this rule that fail 3.1.2 but not 3.1.1.

These two examples are actually pretty much the same

ah, good spot. wanted to make the difference between parent and ancestor clear (or rather, that we're not just talking about parent), but probably no need to do two examples that are so close to each other. will go with ancestor one, as parent is just a special case of ancestor

Note that you can also go even simpler:
check the existence of a lang attribute, not its validity. Then you can go for any ancestor without problem, the rule is much simpler and keep a single concern (it does not need to know what are valid/invalid language tags). Given that we already have rules that check validity of language tags, this is not going to be a real problem.

The point here being that we don't need the rule to be completely covering a single SC. It is better (imho) to have simple rules checking a simple thing and use a combination of rules to cover a SC (rules are already complicated beast, keeping them simple helps writers, readers and testers; simpler rules are also better for separation of concern).

This is a personal opinion and mostly showing what can be done. Of course, go the direction you want 😄 given that you are the one doing the job in the first place…

i'll mull it over. appreciate the thorough discussion on this, particularly as i dove in on the deep end here :)

WilcoFiers · 2020-06-25T08:09:53Z

I'm going to close this as this PR has not been updated in 8 months. If someone wants to pick this up again, we can reopen the PR or create a new one.

patrickhlauke added 2 commits November 21, 2019 10:19

Merge branch 'develop' into element-lang-programmatically-determinable

e3277ac

patrickhlauke mentioned this pull request Nov 21, 2019

"Language of parts" and failure when ancestors/document itself lack language definition #1009

Closed

Jym77 assigned patrickhlauke Nov 21, 2019

Jym77 added the Rule Use this label for a new rule that does not exist already label Nov 21, 2019

Jym77 requested changes Nov 21, 2019

View reviewed changes

patrickhlauke mentioned this pull request Nov 21, 2019

Add Patrick H. Lauke to contributors #1034

Merged

Jym77 reviewed Nov 22, 2019

View reviewed changes

Merge branch 'develop' into element-lang-programmatically-determinable

ea4a62f

Jym77 mentioned this pull request Nov 28, 2019

New rule: Text nodes have minimal contrast #833

Merged

WilcoFiers closed this Jun 25, 2020

New Rule: Element language is programmatically determinable #1027

New Rule: Element language is programmatically determinable #1027

Uh oh!

Conversation

patrickhlauke commented Nov 21, 2019

This will require a 2 weeks Final Call

Pull Request Etiquette

When creating PR:

After creating PR:

How to Review And Approve

Uh oh!

CLAassistant commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickhlauke commented Nov 21, 2019

Uh oh!

patrickhlauke commented Nov 21, 2019

Uh oh!

Jym77 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patrickhlauke commented Nov 21, 2019

Uh oh!

Jym77 commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jym77 commented Nov 21, 2019

Uh oh!

patrickhlauke commented Nov 21, 2019

Uh oh!

Jym77 commented Nov 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickhlauke Nov 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WilcoFiers commented Jun 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Nov 21, 2019 •

edited

Loading

Jym77 commented Nov 21, 2019 •

edited

Loading

patrickhlauke Nov 22, 2019 •

edited

Loading