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

Infrastructure: Update vnu-jar parameters to remove unused paths #2617

Merged
merged 2 commits into from
Jun 26, 2023

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented Feb 14, 2023

See #2637

Update the path used with vnu-jar to content.


WAI Preview Link failed to build on 'Update site files' step. (Last tried on Wed, 21 Jun 2023 14:18:40 GMT).

@howard-e howard-e added the Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation label Feb 14, 2023
@howard-e howard-e requested a review from alflennik February 14, 2023 15:35
@howard-e
Copy link
Contributor Author

The warnings I now get when running this are:

"file:/path/to/aria-practices/content/patterns/treeview/examples/treeview-navigation.html":74.13-74.34: info warning: The “banner” role is unnecessary for element “header”.
"file:/path/to/aria-practices/content/patterns/treeview/examples/treeview-navigation.html":295.17-295.58: info warning: Consider using the “h1” element as a top-level heading only (all “h1” elements are treated as top-level headings by many screen readers and other tools).
"file:/path/to/aria-practices/content/patterns/treeview/examples/treeview-navigation.html":304.13-304.39: info warning: The “contentinfo” role is unnecessary for element “footer”.
"file:/path/to/aria-practices/content/patterns/menubar/examples/menubar-navigation.html":68.13-68.34: info warning: The “banner” role is unnecessary for element “header”.
"file:/path/to/aria-practices/content/patterns/menubar/examples/menubar-navigation.html":216.17-216.58: info warning: Consider using the “h1” element as a top-level heading only (all “h1” elements are treated as top-level headings by many screen readers and other tools).
"file:/path/to/aria-practices/content/patterns/menubar/examples/menubar-navigation.html":226.13-226.39: info warning: The “contentinfo” role is unnecessary for element “footer”.
"file:/path/to/aria-practices/content/shared/templates/example-usage-warning.html":8.9-8.11: error: Element “p” not allowed as child of element “summary” in this context. (Suppressing further errors from this subtree.)

@howard-e
Copy link
Contributor Author

howard-e commented Mar 1, 2023

Going back through the history, the only new (and blocking) error here is: example-usage-warning.html":8.9-8.11: error: Element “p” not allowed as child of element “summary” in this context. (Suppressing further errors from this subtree.)

@nschonni
Copy link
Contributor

nschonni commented Mar 1, 2023

Seems like there is still an issue with the problem matcher for error level messages, since it's not adding the annotation to that file

Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

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

Looks good Howard, and as for the error you're seeing, I tested removing the p tag inside the summary tag and it didn't have any adverse effects and visually looks and behaves identically, so I think removing it would be a good resolution to that error.

@howard-e
Copy link
Contributor Author

howard-e commented Mar 2, 2023

Seems like there is still an issue with the problem matcher for error level messages, since it's not adding the annotation to that file

Unfortunate. Seems similar to what's being tracked in #2082

@howard-e
Copy link
Contributor Author

howard-e commented Mar 2, 2023

I tested removing the p tag inside the summary tag and it didn't have any adverse effects and visually looks and behaves identically, so I think removing it would be a good resolution to that error.

Thanks for taking a look @alflennik. I updated example-usage-warning.html with that change. That lint check now passes.

@howard-e
Copy link
Contributor Author

howard-e commented Mar 7, 2023

This build is now passing following #2640 merge

cc @mcking65 @alflennik

@mcking65
Copy link
Contributor

@howard-e @alflennik

This looks good to me. Should I merge?

I'm wondering if it is possible to modify .vnurc to suppress the warnings from landmarks and H1 in:

  • menubar/examples/menubar-navigation.html
  • treeview/examples/treeview-navigation.html

We don't want to suppress those warnings globally, just for those files.

@howard-e howard-e force-pushed the update-vnu-jar-path branch from f64a834 to e911b75 Compare June 21, 2023 14:13
@howard-e
Copy link
Contributor Author

I'm wondering if it is possible to modify .vnurc to suppress the warnings from landmarks and H1 in:

  • menubar/examples/menubar-navigation.html
  • treeview/examples/treeview-navigation.html

We don't want to suppress those warnings globally, just for those files.

It doesn't seem like this is possible. The path information doesn't seem to be included in what the regex is being written against in .vnurc.

validator/validator#1044 also requested this as a feature and the author has since marked it as an upcoming enhancement.

This looks good to me. Should I merge?

Sure! Unless we want to reconsider globally supressing those warnings for now and tracking it in an issue?

@mcking65 mcking65 changed the title Infrastructure: Update vnu-jar usage Infrastructure: Update vnu-jar configuration to remove unused paths Jun 26, 2023
@mcking65 mcking65 changed the title Infrastructure: Update vnu-jar configuration to remove unused paths Infrastructure: Update vnu-jar parameters to remove unused paths Jun 26, 2023
@mcking65 mcking65 merged commit 83d1329 into main Jun 26, 2023
@mcking65 mcking65 deleted the update-vnu-jar-path branch June 26, 2023 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infrastructure: HTML validator paths are outdated
4 participants