Skip to content

Conversation

@Brynanders
Copy link
Collaborator

<< Describe the changes >>

Closes issue: #216

Guidance for the PR (pull request) creator

When creating PR:

  • Make sure you requesting to pull a issue/feature/bugfix branch (right side) to the master 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 (more to the administrative side)
  • Add relevant project (e.g. "Q3 2018 Status") to PR
  • OPTIONAL: If you want anyone in particular to review your pull request, assign them as "Reviewers".
  • Close the issue that the PR resolves (and make sure the issue is referenced in the top of this comment)

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

@Brynanders Brynanders added reviewer wanted Rule Use this label for a new rule that does not exist already labels Sep 18, 2018
@Brynanders Brynanders self-assigned this Sep 18, 2018
@Brynanders Brynanders changed the title Create SC1-1-1-accessible-name-is-image-file-name.md Create SC1-1-1-accessible-name-is-image-file-name Sep 18, 2018
kasperisager
kasperisager previously approved these changes Sep 18, 2018
Copy link
Collaborator

@nitedog nitedog left a comment

Choose a reason for hiding this comment

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

Need to resolve the filename referencing issue.

Simplified applicability
Changed 1st inapplicable test case.
Added 2nd inapplicable test case
@Brynanders Brynanders dismissed stale reviews from carlosapaduarte and nitedog September 26, 2018 13:59

updated applicability

@nitedog

This comment has been minimized.

@Brynanders Brynanders changed the title Create SC1-1-1-accessible-name-is-image-file-name Create SC1-1-1-alt-name-is-image-file-name Sep 26, 2018
Updated title and description to target `alt` instead of accessible name
@Brynanders

This comment has been minimized.

@nitedog

This comment has been minimized.

@Brynanders

This comment has been minimized.

@Brynanders Brynanders changed the title Create SC1-1-1-alt-name-is-image-file-name Create SC1-1-1-accessible-name-is-image-file-name Sep 26, 2018
@kasperisager

This comment has been minimized.

@nitedog
Copy link
Collaborator

nitedog commented Sep 27, 2018

I don't have data to agree or disagree with that but it seems to me that if authoring tools will rarely use other approaches, like aria-label, then the noise ration will hardly increase. But again, others may have better insights on how often this occurs. In any case, if we do limit this, we should state why in a note or in the assumptions section.

---
name: image accessible name contains image file name
description: |
This rule checks that the use of an image's file name in it's accessible name, is appropriate.
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we put something about authoring tools in the description. Make it clear why this is a thing.

The rule applies to any HTML `img` element that is exposed to assistive technologies, where the accessible name of the element contains the file name specified in the `src` attribute of the element.

### Expectation
Each test target has an accessible name that serves as an appropriate text alternative as described in [F30: Failure of Success Criterion 1.1.1 and 1.2.1 due to using text alternatives that are not alternatives (e.g., filenames or placeholder text)](https://www.w3.org/TR/WCAG20-TECHS/F30.html).
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 unsure if linking to outside tests is allowed in ACT RF. I suggest we make whatever part we want from F30 part of this expectation, rather than us referencing a document that can change at any time.

@Brynanders
Copy link
Collaborator Author

Brynanders commented Sep 27, 2018

@WilcoFiers thanks for your comments and reviews - before I change the rule drastically again I want to get some consensus on targeting alt attributes vs accessible names in general.

@nitedog, @WilcoFiers and @kasperisager if we are going to match exact file names e.g. "example.jpg" instead of "example" or ".jpg" then what would be the harm in including other accessible names for img elements? I hear what you are saying Wilco that authoring tools do not typically populate aria-label for example but so what... if they don't then great, no extra noise, if they do at some stage then great, we catch the issue, it seems like win win to me.

Based on this I propose we target HTML elements with the semantic role of img where the accessible name exactly matches the image file name (with an example), or the image scr attribute value.

Any objections?

Copy link
Collaborator

@ShadowBB ShadowBB left a comment

Choose a reason for hiding this comment

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

Fixed some spelling mistakes. Approved apart from that.

@goetsu
Copy link

goetsu commented Feb 25, 2019

Image a situation where user can download multiple format of the same image

<button><img src="/image.jpg" alt="image.jpg file"></button>
<button><img src="/image.gif" alt="image.gif file"></button>
<button><img src="/image.png" alt="image.png file"></button>

In that case I think using filename is compliant with 1.1 and must not be marked as a failure so this test can be completely automated

@Brynanders
Copy link
Collaborator Author

Corb O'Connor comments in the CFC email chain:

This looks great; thanks for your hard work on this. Am I understanding correctly that the rule requires the extension to be part of the filename? Imagine I have a picture of Barack Obama, saved as “Barack Obama.jpg” but set the ALT to the filename minus the extension, then this rule would not fail…right? Assuming so, then I have no comments!

@Brynanders
Copy link
Collaborator Author

Image a situation where user can download multiple format of the same image

<button><img src="/image.jpg" alt="image.jpg file"></button>
<button><img src="/image.gif" alt="image.gif file"></button>
<button><img src="/image.png" alt="image.png file"></button>

In that case I think using filename is compliant with 1.1 and must not be marked as a failure so this test can be completely automated

@goetsu thanks for your comment.

Your example would not be marked as a failure with this check because the alt "serves an equivalent purpose to the non-text content".

You could write a rule that looks for that exact scenario and fully automate it but I don't think that building rules for a single purpose would be the most efficient approach.

@Brynanders
Copy link
Collaborator Author

Corb O'Connor comments in the CFC email chain:

This looks great; thanks for your hard work on this. Am I understanding correctly that the rule requires the extension to be part of the filename? Imagine I have a picture of Barack Obama, saved as “Barack Obama.jpg” but set the ALT to the filename minus the extension, then this rule would not fail…right? Assuming so, then I have no comments!

In this instance, the accessible name of "Barack Obama" does not include the file extension which is part of the filename specified in the src attribute so it would be considered inapplicable and not be flagged.

Serving only an aesthetic purpose, providing no information, and having no functionality.
Serving only an aesthetic purpose, providing no information, and having no functionality.

**Note:** Authors can mark an `img` element as decorative to indicate that it should be ignored by assistive technology by using either `role="presentation"`, `role="none"`, or `alt=""`. An element should only be marked as decorative if removing the element does not cause a loss of information to the user.
Copy link
Collaborator

@EmmaJP EmmaJP Feb 26, 2019

Choose a reason for hiding this comment

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

Are there any elements other than 'img' that could be marked as decorative, such as 'svg'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The context of this rule is img so the note addresses that specifically and not other elements.

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 could change this to say "Authors can mark elements as decorative..." if you think that would make more sense?

Copy link
Collaborator

@EmmaJP EmmaJP left a comment

Choose a reason for hiding this comment

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

OK with this apart from my question on the 'decorative' definition. Seems very thorough.

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.

Some editorial issues yet, but rule is looking good

@WilcoFiers WilcoFiers added Review call 2 weeks Call for review for new rules and big changes and removed Agenda item labels Feb 28, 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.

Thank you for putting this in the CFC. I'm afraid I found some problems with the way this is done. I don't think your test cases are actually applicable. It seems to me the applicability is missing some of its key parts.


## Test Cases

### Passed
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't seem like any of the passed / failed examples would be applicable. The applicability doesn't say to ignore file extensions or its file path.

Copy link
Collaborator Author

@Brynanders Brynanders Mar 4, 2019

Choose a reason for hiding this comment

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

@WilcoFiers the applicability refers to a definition of "filename" https://github.com/auto-wcag/auto-wcag/blob/master/pages/glossary/filename.md which excludes the file path and appended query strings.

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 @kasperisager and I have reworked some of the test cases to fit the applicability. Please can you give this another look.

@Brynanders Brynanders requested a review from WilcoFiers March 4, 2019 13:00
@Brynanders Brynanders dismissed WilcoFiers’s stale review March 5, 2019 12:56

Updated test cases

@annethyme
Copy link
Collaborator

@Brynanders, I think that since Carlos' permissions changed the other day, the status of this PR is now "changes requested" instead of "approved". I think you will have to dismiss Carlos' (old) review to get things back in order.

@annethyme
Copy link
Collaborator

@Brynanders, @JKODU, @WilcoFiers, can this one be merged?
It has survived its two-week CF.

@Brynanders Brynanders merged commit b1e8b26 into master Mar 26, 2019
@Brynanders Brynanders deleted the Brynanders-patch-2 branch March 26, 2019 09:43
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agenda item Review call 2 weeks Call for review for new rules and big changes Rule Use this label for a new rule that does not exist already

Projects

None yet

Development

Successfully merging this pull request may close these issues.