Infrastructure: link-checker.js: Check if any of the known pageIds include the hash being checked for
#2812
Conversation
howard-e
added
the
Infrastructure
scripts/link-checker.js
Outdated
| @@ -227,7 +227,7 @@ async function checkLinks() { | |||
|
|
|||
| let matchesHash = true; | |||
| if (hash) { | |||
| matchesHash = !!matchingPage?.ids.includes(hash); | |||
| matchesHash = !!matchingPage?.ids.some((id) => id.includes(hash)); | |||
There was a problem hiding this comment.
Since it's not checking that the ID is exact I think an explanatory comment might be warranted, especially one that explains that this reproduces the behavior already present in GitHub.
scripts/link-checker.js
Outdated
| // as being user-content-foo-bar for its own page processing purposes. | ||
| // | ||
| // See https://github.com/w3c/aria-practices/issues/2809 | ||
| matchesHash = !!matchingPage?.ids.some((id) => id.includes(hash)); |
There was a problem hiding this comment.
I think this condition is not sufficiently restrictive. It makes it too easy for me (and others) to create href attributes that are not valid. I really want to be confident that our content does not contain invalid fragment ids. Even though there is the possibility of more maintenance to this code, I'd rather have the condition be specific to GitHub pages and the "user-content-" prefix. It does not seem likely we will encounter this problem on other pages referenced by the APG.
Over time, if it turns out GitHub is wishywashy in there approach or if this turns out to be a problem with other sites, we can decide how to address those other scenarios when they arise.
There was a problem hiding this comment.
@mcking65 I've updated the code to account specially account for this GitHub scenario.
I also set it up to handle any others in the future using the .link-checker.js file which seems to handle some config settings.
There was a problem hiding this comment.
cc @alflennik I also removed /^https:\/\/github\.com\/.*\/wiki\// from ignoreHashesOnExternalPagesMatchingRegex as the same situation would happen on those wiki pages.
|
Thank you @howard-e and @alflennik |
Closes #2809. Follow up on my thoughts in #2809 (comment).
Should this explicitly check for
user-content-when checking GitHub links instead?WAI Preview Link (Last built on Mon, 02 Oct 2023 13:59:20 GMT).