Invalid Links Rule #19
Conversation
There was a problem hiding this comment.
Actually sorry @Synvox the issue wasn't super clear, but we're looking for something a little bit more robust.
This is definitely good though :)
So additionally we'd like to have it validate that the content it is linking to works. So if navigating to the link would return an error condition to the user, we'd want it to alert about that.
src/rules/valid-links.js
Outdated
| "Invalid links become broken links which are confusing to all users." | ||
| ), | ||
|
|
||
| link: "https://www.w3.org/TR/2016/NOTE-WCAG20-TECHS-20161007/H48" |
There was a problem hiding this comment.
Also, this link isn't linking to the proper content.
There was a problem hiding this comment.
Overall really strong work. There are some unanswered questions I'll work with @DanaDangerGrey to track down the answers to for you.
package.json
Outdated
| @@ -63,6 +63,7 @@ | |||
| "axios": "^0.18.0", | |||
| "format-message": "^6", | |||
| "format-message-generate-id": "^6", | |||
| "node-fetch": "^2.1.2", | |||
There was a problem hiding this comment.
We already have axios in place. It should be able to handle this same sort of thing without needing an additional library involved.
There was a problem hiding this comment.
Oh sweet. I'll just use that then.
src/rules/valid-links.js
Outdated
| .then(res => resolve(res.ok)) | ||
| .catch(() => resolve(false)) | ||
|
|
||
| // This will always work in jest because there |
There was a problem hiding this comment.
This makes sense, the limitations with CORS requests from AJAX definitely cause issues here. I'm not sure we want to deal with making a service to handle this or the complexity of introducing service workers to a plugin like this.
I'll see about getting some clarification on how we want to proceed.
There was a problem hiding this comment.
@Synvox I've talked with some stakeholders internally on how this should go... for now.. you can just go with your original "link schema checker" rather than the full on "link validator"
There was a problem hiding this comment.
That's what I was thinking. It is possible to check same origin links but much more complicated to do cross-origin. There may be a way to reference a foreign resource through an <img> tag or similar, and check to see if the image fails to load because of a failing status code. It's hacky, but similar to how JSONP works.
There was a problem hiding this comment.
That's true. If you wanna do some experimentation with that... that could be really cool. We do something similar to that to detect broken images in canvas-lms by adding a onerror handler to images. Kinda hacky, but maybe workable to do something like that here.(https://github.com/instructure/canvas-lms/blob/master/app/coffeescripts/behaviors/broken-images.js)
There was a problem hiding this comment.
Here's a thought. https://codepen.io/Synvox/pen/pZvYPb?editors=0012
It looks like IMG tags don't report their failure reason which makes it impossible to detect if it failed because of a 404, or if it failed because it tried to load a document instead of an image.
The alternative is to load the resource in a Worker. The worker only throws when the resource fails on the network, which is exactly what we need.
There was a problem hiding this comment.
Looking at that... it seems like a good solution... they aren't supported in IE 11, but I think we could probably just let that be a restriction on the plugin. If you aren't in IE then you get a little bit more validation, otherwise you get the basic stuff.
| return elem.parentNode | ||
| }, | ||
|
|
||
| // Note, these messages are poor and should be replaced with |
There was a problem hiding this comment.
I'll get some words for you here :)
Fixes #7