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.

Sign up for GitHub

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

Jump to bottom

trailing/leading spaces in roles cause issues #464

Closed
jnurthen opened this issue Feb 16, 2022 · 3 comments · Fixed by #467
Closed

trailing/leading spaces in roles cause issues #464

jnurthen opened this issue Feb 16, 2022 · 3 comments · Fixed by #467
Labels
bug
Milestone
next

Comments

@jnurthen
Copy link

role="main " is not recognized as a landmark.
Trailing and leading spaces should all be allowed as roles are space-separated tokens in HTML https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#space-separated-tokens

Further role="main foo" and role="foo main" (assuming foo isn't a known role in the latter case) should also both resolve to main in the landmarks extension.
@matatk
Copy link
Owner

matatk commented Feb 18, 2022

Great catch; thanks @jnurthen!

@matatk matatk added the bug label Feb 18, 2022
@matatk matatk added this to the next milestone Feb 18, 2022
@matatk
Copy link
Owner

matatk commented Mar 24, 2022

Quick update, as it's been a while!

I have fixed this on my local machine; just have a few performance tests to run. I'm expecting to complete that over the weekend, and push new versions of both the test suite and this extension (it'll take a while after that for the extension to be reviewed by the browsers; usually that's not more than a few days).

matatk added a commit to matatk/page-structural-semantics-scanner-tests that referenced this issue Mar 27, 2022
@matatk
feat: Add "role" attribute token list tests
dec6716 
Originated by matatk/landmarks#464 - thanks :-)
matatk added a commit to matatk/page-structural-semantics-scanner-tests that referenced this issue Mar 27, 2022
@matatk
feat: Add "role" attribute token list tests
d9815c1 
Originated by matatk/landmarks#464 - thanks :-)
matatk added a commit to matatk/page-structural-semantics-scanner-tests that referenced this issue Mar 27, 2022
@matatk
feat: Add "role" attribute token list tests
922096a 
Originated by matatk/landmarks#464 - thanks :-)
matatk added a commit to matatk/page-structural-semantics-scanner-tests that referenced this issue Mar 27, 2022
@matatk
feat: Add "role" attribute token list tests
470a096 
Originated by matatk/landmarks#464 - thanks :-)
@matatk matatk mentioned this issue Mar 27, 2022
Merged
matatk added a commit to matatk/page-structural-semantics-scanner-tests that referenced this issue Mar 27, 2022
@matatk
feat: Add "role" attribute token list tests (#36)
a66ea9f 
Originated by matatk/landmarks#464 - thanks :-)
@matatk matatk mentioned this issue Mar 28, 2022
Merged
matatk added a commit that referenced this issue Mar 28, 2022
@matatk
fix: Support the "role" attribute correctly as a token list (#467)
2ca800a 
* Handle the role attribute as a token list.
* Bump page-structural-semantics-scanner-tests to include token list
  tests.
* Don't check for valid roles twice (checked performance; seems
  negligible, but definitely was duplicated work).
* Move some of the functions around so that definition order matches
  order called in getLandmarks().
* Add a couple of TODO notes.

Fixes #464
@matatk
Copy link
Owner

matatk commented Apr 1, 2022

2.11.1 has been released and contains the fix for this bug. It's available in Firefox and Chrome now; Opera and Edge should follow in several days, after review. Thanks again for your report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
next
Development

Successfully merging a pull request may close this issue.

fix: Support the "role" attribute correctly as a token list matatk/landmarks
2 participants
@matatk @jnurthen