-
Notifications
You must be signed in to change notification settings - Fork 44
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
[SONARHTML-214] Add s6821 #302
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some minor comments
sonar-html-plugin/src/main/java/org/sonar/plugins/html/api/HtmlConstants.java
Outdated
Show resolved
Hide resolved
if (role == null) { | ||
return; | ||
} | ||
var values = role.split(" "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the role attribute have multiple values? if so, we should revisit implementation of other rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericmorand-sonarsource thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems that way - https://www.w3.org/TR/2014/REC-wai-aria-20140320/host_languages#host_general_role
The attribute [value](https://www.w3.org/TR/2014/REC-wai-aria-20140320/terms#def_value) MUST allow a token list as the value;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is confirmed by the specification of the attribute itself: https://www.w3.org/TR/role-attribute/#s_role_module_attributes
The role attribute takes as its value one or more whitespace separated TERMorCURIEorAbsIRIs, which is defined in [RDFA-CORE]
Quality Gate passedIssues Measures |
https://sonarsource.atlassian.net/jira/software/c/projects/SONARSWIFT/boards/406?selectedIssue=SONARHTML-214
Mimicking logic from: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/rules/aria-role.js
Not including the options for now, as there also aren't any defaults in the original implementation.
The 1 thing I worry is about the templating languages hence using getAttribute vs getProperty to find mostly vanilla calls of : <... rule=""> instead of the vue or angular style attributes, where they can pass variables.