Add rule to check the usage of the Content-Type HTTP response header
#245
Conversation
| description: 'Check usage of `Content-Type` HTTP response header' | ||
| }, | ||
| fixable: 'code', | ||
| ignoredCollectors: ['cdp'], // TODO: Remove once #71 and #164 are fixed. |
There was a problem hiding this comment.
OK, so what do you need CDP to download automatically to work? favicon.ico always?
There was a problem hiding this comment.
so what do you need CDP to download automatically to work?
@molant Either jsdom and CDP both download the favicon.ico, or none do. Right now only CDP sometimes downloads it.
|
|
||
| if (nodeName) { | ||
|
|
||
| if (nodeName === 'script') { |
There was a problem hiding this comment.
What happens if people are using script tags for other things like templates? Maybe we should check the type (or its absence)?
In the cases I've found the HTML is directly into the script tag so no request will be done (angular, handlebars) but I'm not sure if there is another template or library that downloads something automatically.
E.g.:
<script type="custom/template" src="/mycustomtemplate.html"></script>And then my custom template library does some magic and downloads the content.
There was a problem hiding this comment.
And then my custom template library does some magic and downloads the content.
Funny thing: Firefox and Edge download the content, Chrome doesn't.
There was a problem hiding this comment.
FYI: Even if <script type="type/subtype" src="file.extension"></script> is used, if the file content or file extension suggests a different type, I think we should suggest the appropriate media type for that file.
Also, I don't think there will be a lot of cases here, usually <script type="type/subtype">content...</script> is what is used, plus even that should slowly be replaced with the use of <template>.
| .pop(); | ||
|
|
||
| return getMediaTypeBasedOnFileExtension(fileExtension); | ||
|
|
There was a problem hiding this comment.
Can we refactor this a bit so this method isn't that long?
I think we should have a separate function per detection strategy and then execute them until one works.
Something similar to:
const detections = [nodeName, fileContent, fileExtension];
const result = detections.some((detection) => {
return detection(element, resource, body);
});| description: 'Check usage of `Content-Type` HTTP response header' | ||
| }, | ||
| fixable: 'code', | ||
| ignoredCollectors: ['cdp'], // TODO: Remove once #71 and #164 are fixed. |
There was a problem hiding this comment.
OK, so what do you need CDP to download automatically to work? favicon.ico always?
|
@molant This is ready for another review. |
Fix #141