Skip to content
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

Rule update: "HTML page has a title" #440

Merged
merged 11 commits into from Apr 10, 2019

Conversation

annethyme
Copy link
Collaborator

"contain" --> "descendant" in Expectation 1.
Editorial cleanup of "Background" links

Closes issue: #316

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

"contain" --> "descendant" in Expectation 1. Editorial cleanup of "Background"
@annethyme annethyme added Rule Update Use this label for an existing rule that is being updated Rule Use this label for a new rule that does not exist already March 31 deadline labels Mar 14, 2019
@annethyme annethyme self-assigned this Mar 14, 2019
@annethyme annethyme added this to Needs Review in Rules Progress via automation Mar 14, 2019
@@ -28,13 +28,13 @@ The rule applies to any page where the root element is an `html` element, and wh

### Expectation 1

The page contains at least one `title` element.
The root element has at least one [descendant](https://www.w3.org/TR/dom41/#concept-tree-descendant) that is an HTML `title` element.
Copy link
Contributor

Choose a reason for hiding this comment

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

@annethyme annethyme dismissed kasperisager’s stale review March 18, 2019 14:56

Now linking to definition of "root" in both Applicability and Expectation

kasperisager
kasperisager previously approved these changes Mar 18, 2019
audreymaniez
audreymaniez previously approved these changes Mar 20, 2019
Copy link
Collaborator

@audreymaniez audreymaniez left a comment

Choose a reason for hiding this comment

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

Ok! I just add a suggestion about a technique.

- [https://www.w3.org/TR/WCAG20-TECHS/H25.html](https://www.w3.org/TR/WCAG20-TECHS/H25.html)
- The WCAG 2.0 Techniques already contain examples and code snippets to illustrate which content passes or fails the test. Whenever possible {{site.title}} refers to those. Another source for test cases is the W3C Before and After Demonstration.
- [Understanding Success Criterion 2.4.2: Page Titled](https://www.w3.org/WAI/WCAG21/Understanding/page-titled.html)
- [WCAG 2.1 Technique G88: Providing descriptive titles for Web pages](https://www.w3.org/WAI/WCAG21/Techniques/general/G88)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this technique G88 really relate to that rule ? It seems the G88 is for the rule you write in the #380 PR ? I just suggest remove the relation for less confusion on the scope of the rule.


### Expectation 2

The first `title` element contains [non-empty text](#non-empty).
The first `title` element contains [letters or numbers](#letters-or-numbers).
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I understand, this means that the following title would fail, because it only uses punctuation symbols, not letters or numbers?
<title>#$@&%*!</title>
And should it fail?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carlosapaduarte, yes, with the current definition it would. The old "non-empty" definition was actually also only looking for letters and numbers, it was just much less obvious.

I have struggled with the same question for headings. If I create a page about smileys, I would say that ":-)" and ":'-(" would be fine as headings.

What do you think? Another definition that I have used in some places is "contains text nodes that do not only consist of Unicode separator characters"
So <title>#$@&%*!</title> would be a Passed outcome, but <title> </title>would be Failed.

Do you think that would be more appropriate?

We do also have another rule that checks if the title is descriptive, so I think I am actually leaning towards this approach...

Copy link
Member

Choose a reason for hiding this comment

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

@annethyme, yes I do prefer the "contains text nodes that do not only consist of Unicode separator characters" definition.

I don't think that both the #$@&%*! title and the :-) heading are accessibility violations, so they shouldn't be failed by a rule that checks just a syntactic aspect (in this case, if the page has a title or not).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Change has been made and test cases updated to reflect this.

@annethyme annethyme dismissed kasperisager’s stale review March 21, 2019 10:10

Substantial changes have been made since approval, so I will dismiss the review.

@annethyme annethyme dismissed audreymaniez’s stale review March 21, 2019 10:10

Substantial changes have been made since approval, so I will dismiss the review.

@@ -22,19 +22,19 @@ authors:

### Applicability

The rule applies to any page where the root element is an `html` element, and which is not embedded in another page.
The rule applies to any page where the [root](https://www.w3.org/TR/dom41/#concept-tree-root) element is an `html` element, and which is not embedded in another page.
Copy link
Contributor

Choose a reason for hiding this comment

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

The root of a document will always be the document itself so no valid HTML documents will ever match this applicability. The term we're looking for is "document element": https://www.w3.org/TR/dom/#document-element

Copy link
Member

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

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

Just a couple of typos in addition to what Kasper has raised

@@ -166,11 +174,21 @@ Empty first `title`.
</html>
```

#### Failed example 5

The `title` only contains a seperator character.
Copy link
Member

Choose a reason for hiding this comment

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

typo: seperator -> separator

Above there is another typo. On passed example 2 "This page give" -> "This page gives"

@annethyme annethyme dismissed kasperisager’s stale review March 22, 2019 10:10

Should be fixed now.

Rules Progress automation moved this from Needs Review to CFC (Reviewer Approved) Mar 25, 2019
@annethyme annethyme added Agenda item Review call 2 weeks Call for review for new rules and big changes and removed Agenda item labels Mar 26, 2019
annethyme pushed a commit that referenced this pull request Mar 27, 2019
Copy link
Member

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Super editorial, but please consider :)


### Expectation 2

The first `title` element contains [non-empty text](#non-empty).
The first `title` element contains [text nodes](https://www.w3.org/TR/dom/#text) that do not only consist of [Unicode separator characters](https://www.unicode.org/versions/Unicode11.0.0/ch04.pdf#G134153).
Copy link
Member

Choose a reason for hiding this comment

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

Can we simplify this a little by doing:

The first title element contains text that does not only consist of whitespace (specifically, Unicode separator characters).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@WilcoFiers, we don't find your suggestion to be as unambiguous as we would like.

Copy link
Member

Choose a reason for hiding this comment

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

Which part of it? Alternative proposal:

... contains text content that does not only consist of Unicode separator characters (i.e. space, line breaks and other whitespace characters)

Text content is what the HTML 5.2 spec says needs to be used to derive the document title from the title element, so it's the right technical term, and it reads better than "text nodes" IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

HTML defines "child text content" as a term (https://www.w3.org/TR/html/infrastructure.html#child-text-content), not "text content". In general, I don't really think it's appropriate to refer do DOM attributes in the manner proposed, e.g. use "text content" to mean node.textContent or "document title" to mean document.title. If referring to DOM attributes or properties, be explicit about it.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point. But then shouldn't this simply be using document.title?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it should; that's an implementation detail and rules shouldn't dictate the "how" but rather the "what". The "what", in this case, is the concatenation of the data of the text nodes that are children of the <title> element, i.e. the "child text content". The "how" could be document.title.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mean to say to literally access the document.title property. You're right, that's clearly implementation. I mean using "document title", linking to the algorithm for deriving it, the same way we talk about "accessible name" and referencing that algorithm.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes perfect sense and for that very same reason I'd also suggest using the term "child text content" similar to how we use the term "accessible name". We could even add "document title" to the glossary and define it as "the child text content of the first <title> element that is a descendant of the document".

Rules Progress automation moved this from CFC (Reviewer Approved) to Needs Review Mar 27, 2019
@annethyme annethyme dismissed WilcoFiers’s stale review April 1, 2019 11:12

Changed to match Kasper's suggestion for a resolution.

Rules Progress automation moved this from Needs Review to CFC (Reviewer Approved) Apr 1, 2019
Rules Progress automation moved this from CFC (Reviewer Approved) to Needs Review Apr 1, 2019

### Expectation 2

The first `title` element contains [non-empty text](#non-empty).
The [child text content](https://www.w3.org/TR/html/infrastructure.html#child-text-content) of the first HTML `title` element that is a [descendant](https://www.w3.org/TR/dom41/#concept-tree-descendant) of the [document element](https://www.w3.org/TR/dom/#document-element) does not only consist of [Unicode separator characters](https://www.unicode.org/versions/Unicode11.0.0/ch04.pdf#G134153).
Copy link
Member

Choose a reason for hiding this comment

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

Please use plain language here. We can replace "non-empty" as a definition, but we shouldn't be inlining complex definitions like this. That makes this very difficult to use for less technical people.

Copy link
Contributor

Choose a reason for hiding this comment

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

@annethyme annethyme dismissed WilcoFiers’s stale review April 4, 2019 12:45

Wording changed. Please review again.

Rules Progress automation moved this from Needs Review to CFC (Reviewer Approved) Apr 4, 2019

### Expectation 2

The first `title` element contains [non-empty text](#non-empty).
The first HTML `title` element that is a [descendant](https://www.w3.org/TR/dom41/#concept-tree-descendant) of the [document element](https://www.w3.org/TR/dom/#document-element) has [children](https://www.w3.org/TR/dom/#concept-tree-child) that are [text nodes](https://www.w3.org/TR/dom/#text) that are not only [whitespace](#whitespace).

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@annethyme annethyme merged commit 0585a59 into master Apr 10, 2019
Rules Progress automation moved this from CFC (Reviewer Approved) to Done Apr 10, 2019
kasperisager added a commit that referenced this pull request Apr 10, 2019
* master:
  Rule update: "HTML page has a title" (#440)
  Rule update: "aria attribute allowed" and "aria required states and properties" (#436)
  Changed succescriteria from 4.1.2 to 1.3.1 (#449)
  Rename SC1-2-Video-description-track.md to SC1-2-video-description-track.md
  Glossary: add "Accessibility Support" to "Semantic role" term (#442)
  SC1-1-1-filename-is-valid-accessible-name (#263)
  Added assumption for SC3-1-2-lang-valid (#413)
  Update SC4-1-1-unique-id.md
  fix: update applicability
  fix: revert definitions
  fix: update glossary
  fix: applicability
kasperisager added a commit that referenced this pull request Apr 26, 2019
* develop: (76 commits)
  Update SC1-1-1-image-has-name.md (#468)
  chore: update test case names to be hash and not have outcome (#485)
  chore: update styles
  chore: fix styling and spacing on various resolutions
  chore: fix typo in rules
  chore: update dependencies
  chore: add unique id to all rules (#478)
  fix: move footer from nav
  fix: update cache version
  fix: bust dependency cache
  chore: WCAG ACT RULES CG Website Update (#437)
  chore: update site publish config
  Replace auto-wcag with act-r in readme
  Update _config.yml
  Rule update: "aria-hidden with focusable content" (#439)
  Rule update: "HTML page has a title" (#440)
  Rule update: "aria attribute allowed" and "aria required states and properties" (#436)
  Changed succescriteria from 4.1.2 to 1.3.1 (#449)
  Rename SC1-2-Video-description-track.md to SC1-2-video-description-track.md
  Glossary: add "Accessibility Support" to "Semantic role" term (#442)
  ...
@annethyme annethyme moved this from Done to 3 implementations in Rules Progress Jul 18, 2019
@jeeyyy jeeyyy deleted the rule-update-SC-2-4-2-page-has-title branch July 25, 2019 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review call 2 weeks Call for review for new rules and big changes Rule Update Use this label for an existing rule that is being updated Rule Use this label for a new rule that does not exist already
Projects
No open projects
Rules Progress
  
3 implementations
Development

Successfully merging this pull request may close these issues.

None yet

7 participants