fix(ngAria): Prevent aria-invalid from being added to hidden inputs #15113

edclements · 2016-09-09T14:54:57Z

This fixes a error found using the Google Accessibility Developer Tools audit.
Input fields of type hidden shouldn't have aria attributes.
https://www.w3.org/TR/html-aria/#allowed-aria-roles-states-and-properties-1

This fixes a error found using the Google Accessibility Developer Tools audit. Input fields of type hidden shouldn't have aria attributes. https://www.w3.org/TR/html-aria/#allowed-aria-roles-states-and-properties-1

googlebot · 2016-09-09T14:55:01Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

googlebot · 2016-09-09T14:55:01Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

edclements · 2016-09-09T14:56:16Z

I signed it!

gkalpak · 2016-09-09T19:50:33Z

Input fields of type hidden shouldn't have aria attributes.

We are already excluding all input fields as part of the !isNodeOneOf(...) check.
I am going to close this, since everything seems to be working as expected, but feel free to continue the discussion below.

BTW, the section of the spec you have linked to does not refer to what your PR is about. It refers to elements with a hidden attribute (e.g. <div hidden>).

edclements · 2016-09-10T11:56:12Z

The isNodeOneOf check doesn't apply for aria-invalid as allowBlacklistEls is being passed in as true. The test I wrote will fail without the code change.

The relevant row in the spec reads as follows:
input type = hidden No corresponding role No role or aria* attributes

gkalpak · 2016-09-10T13:20:02Z

Sorry, I read this incorrectly. I am reopening and will review soon.
In the meantime, make sure you have signed the CLA (or we won't be able to merge this).

gkalpak · 2016-09-10T18:31:05Z

test/ngAria/ariaSpec.js

@@ -420,6 +420,11 @@ describe('$aria', function() {
      scope.$apply('txtInput=\'LTten\'');
      expect(element.attr('aria-invalid')).toBe('userSetValue');
    });
+
+    it('should not attach if input is hidden', function() {


Nitpick: It would be more clear to say: ...if input is type="hidden"

gkalpak · 2016-09-10T18:31:53Z

LGTM (but we still need a signed CLA to merge).
@edclements, feel free to ping me when/if you have sorted it out.

edclements · 2016-09-11T11:45:26Z

Thanks for taking a look @gkalpak. I signed the CLA as a company: BookingBug.

gkalpak · 2016-09-12T11:08:34Z

@IgorMinar (or someone else), can you verify the CLA?

IgorMinar · 2016-10-14T19:21:28Z

@gkalpak BookingBug is good! 👍

gkalpak · 2016-10-14T20:04:47Z

Actually, I am going to mark this as a breaking change and merge it into 1.6 only. It might break tests (unlikely but possible).

gkalpak · 2016-10-20T07:17:07Z

We decided not to add more breaking changes into 1.6 at this point, so moving this to 1.7.

Narretz · 2017-01-30T21:12:51Z

LGTM2 and it can be merged because master is on 1.7

gkalpak · 2017-01-31T08:39:41Z

Actually, I just realized that -while the the spec only mentions input[type=hidden] - this PR affects all [type=hidden] elements. @edclements can you update it so that it ignores input element's only and add a corresponding test?

Narretz · 2017-11-30T10:12:24Z

@edclements it's been a while but are you still interested / available to make the changes required for this PR? See #15113 (comment)

@edclements

This fixes a error found by @edclements using the Google Accessibility Developer Tools audit. Input fields of type hidden shouldn't have aria attributes. https://www.w3.org/TR/html-aria/#allowed-aria-roles-states-and-properties-1 Closes angular#15113

Narretz · 2017-12-08T12:51:10Z

I've created an updated PR here: #16367 No need to update this PR anymore. Thanks @edclements for getting this on the road.

@edclements

This fixes a error found by @edclements using the Google Accessibility Developer Tools audit. Input fields of type hidden shouldn't have aria attributes. https://www.w3.org/TR/html-aria/#allowed-aria-roles-states-and-properties-1 Closes #15113 Closes #16367 BREAKING CHANGE: ngAria no longer sets aria-* attributes on input[type="hidden"] with ngModel. This can affect apps that test for the presence of aria attributes on hidden inputs. To migrate, remove these assertions. In actual apps, this should not have a user-facing effect, as the previous behavior was incorrect, and the new behavior is correct for accessibility.

fix(ngAria): Prevent aria-invalid from being added to hidden inputs

bea8457

This fixes a error found using the Google Accessibility Developer Tools audit. Input fields of type hidden shouldn't have aria attributes. https://www.w3.org/TR/html-aria/#allowed-aria-roles-states-and-properties-1

googlebot added the cla: no label Sep 9, 2016

gkalpak closed this Sep 9, 2016

gkalpak reopened this Sep 10, 2016

gkalpak reviewed Sep 10, 2016
View reviewed changes

gkalpak added this to the Purgatory milestone Sep 10, 2016

gkalpak added type: bug frequency: low severity: confusing component: ngAria labels Sep 10, 2016

gkalpak self-assigned this Sep 10, 2016

fix(ngAria): clarify input type="hidden" spec

63bc0a0

IgorMinar added cla: yes and removed cla: no labels Oct 14, 2016

gkalpak modified the milestones: 1.7.0, Purgatory Oct 20, 2016

mgol added the needs: breaking change label Nov 15, 2016

Narretz closed this Nov 30, 2017

Narretz reopened this Nov 30, 2017

Narretz mentioned this pull request Dec 8, 2017

fix(ngAria): do not set aria attributes on input[type="hidden"] #16367

Merged

3 tasks

Narretz closed this Dec 8, 2017

dependencies bot mentioned this pull request May 11, 2018

Update 3 dependencies from npm JetBrains/ring-ui#257

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ngAria): Prevent aria-invalid from being added to hidden inputs #15113

fix(ngAria): Prevent aria-invalid from being added to hidden inputs #15113

edclements commented Sep 9, 2016

googlebot commented Sep 9, 2016

googlebot commented Sep 9, 2016

edclements commented Sep 9, 2016

gkalpak commented Sep 9, 2016 •

edited

Loading

edclements commented Sep 10, 2016

gkalpak commented Sep 10, 2016 •

edited

Loading

gkalpak Sep 10, 2016

gkalpak commented Sep 10, 2016

edclements commented Sep 11, 2016

gkalpak commented Sep 12, 2016

IgorMinar commented Oct 14, 2016

gkalpak commented Oct 14, 2016

gkalpak commented Oct 20, 2016

Narretz commented Jan 30, 2017

gkalpak commented Jan 31, 2017 •

edited

Loading

Narretz commented Nov 30, 2017

Narretz commented Dec 8, 2017

fix(ngAria): Prevent aria-invalid from being added to hidden inputs #15113

fix(ngAria): Prevent aria-invalid from being added to hidden inputs #15113

Conversation

edclements commented Sep 9, 2016

googlebot commented Sep 9, 2016

googlebot commented Sep 9, 2016

edclements commented Sep 9, 2016

gkalpak commented Sep 9, 2016 • edited Loading

edclements commented Sep 10, 2016

gkalpak commented Sep 10, 2016 • edited Loading

gkalpak Sep 10, 2016

Choose a reason for hiding this comment

gkalpak commented Sep 10, 2016

edclements commented Sep 11, 2016

gkalpak commented Sep 12, 2016

IgorMinar commented Oct 14, 2016

gkalpak commented Oct 14, 2016

gkalpak commented Oct 20, 2016

Narretz commented Jan 30, 2017

gkalpak commented Jan 31, 2017 • edited Loading

Narretz commented Nov 30, 2017

Narretz commented Dec 8, 2017

gkalpak commented Sep 9, 2016 •

edited

Loading

gkalpak commented Sep 10, 2016 •

edited

Loading

gkalpak commented Jan 31, 2017 •

edited

Loading