HTML: test stylesheet loading text/css requirement #13144
Conversation
wpt-pr-bot
requested review from
domenic,
foolip,
jdm,
jgraham,
zcorpan and
zqzhang
|
I wrote this for https://bugzilla.mozilla.org/show_bug.cgi?id=562377. It seems this could use some cleanup in HTML as well around the requirements. |
|
(I would have thought we test this elsewhere too, but I couldn't find anything quickly.) |
|
@zcorpan thanks, do you happen to know if we have quirks tests for this? If not, I'll add more tests Monday or so. |
| @@ -44,5 +45,44 @@ | |||
| elt.rel = "stylesheet"; | |||
| elt.href = "../../../../../common/css-red.txt"; | |||
| document.getElementsByTagName("head")[0].appendChild(elt); | |||
| }) | |||
| }); | |||
There was a problem hiding this comment.
nit: Would you be up for adding semicolons above as well to match here, and maybe use t.step_func_done() in the above preexisting tests?
There was a problem hiding this comment.
I try not to make stylistic changes at the same time as that messes with blame too much in my opinion. And then doing it as a separate commit often seems like too much effort... I might not be making the right tradeoffs though.
|
I don't know, but suspect we don't have that. |
|
I added tests for quirks and limited quirks. It seems Chrome and Safari are more relaxed when it comes to style sheet loading. I'll file bugs. |
|
And wow, Edge is even worse... |
|
whatwg/html#3457 is probably the best HTML Standard issue to track improvements at. |
|
I don't think we should block landing this on those happening though. |
|
"limitedd" (in the commit message) isn't how you spell limited. :) Also: 🎉 for having tests for this. |
| @@ -0,0 +1,48 @@ | |||
| <!DOCTYPE HTML PUBLIC "-//Sun Microsystems Corp.//DTD HotJava Strict HTML//" ""> | |||
There was a problem hiding this comment.
Can you replace this with a comment saying "quirks mode"?
| [ | ||
| ["same-origin", "resources/css.py"], | ||
| ["cross-origin", get_host_info().HTTP_REMOTE_ORIGIN + "/html/semantics/document-metadata/the-link-element/resources/css.py"] | ||
| ].forEach(originType => { |
There was a problem hiding this comment.
Please deduplicate the code by moving it into a separate JS file.
There was a problem hiding this comment.
I guess that's reasonable. Do you think it's fine to make it depend on document.compatMode or do you think it should be something you call externally?
The other thing I was thinking of also testing is @import, but how that affects load/error events isn't really defined yet.
There was a problem hiding this comment.
Using document.compatMode should be fine.
|
The test is loading urls like "resources/css.py...." from a sheet that's loaded from a data: URL. Those relative URIs are resolved relative to the data: URL, which fails, so the load is not performed at all. Whether this should trigger the error event is an interesting question, but at the moment the test is not testing what it thinks it's testing. |
No description provided.