Skip to content

Conversation

nt1m
Copy link
Member

@nt1m nt1m commented Aug 9, 2025

@nt1m nt1m requested a review from a team as a code owner August 9, 2025 04:53
@nt1m nt1m self-assigned this Aug 9, 2025
@wpt-pr-bot wpt-pr-bot added the html label Aug 9, 2025
@wpt-pr-bot
Copy link
Collaborator

This patch has been exported from WebKit; it will be approved automatically once the downstream patch is r+.

@nt1m nt1m force-pushed the wpt-export-for-webkit-297161 branch 2 times, most recently from 9b4d64e to 26bb4dc Compare August 9, 2025 05:29
@nt1m
Copy link
Member Author

nt1m commented Aug 9, 2025

@chrishtr @foolip Could you sign off on this change from the Chromium side?

While fixing the only WebKit failure in this test, I found other bugs, and wrote tests for them. Chromium seems to share some of these bugs as well. Firefox passes all the tests written here.

This would bring Chrome's "Modules" score from 100% to ~98.5%, but the failures are fairly straightforward to fix, it would just be checking that the mime type only uses valid HTTP tokens on both sides of the / in the mime type.

See https://mimesniff.spec.whatwg.org/#parse-a-mime-type

@nt1m
Copy link
Member Author

nt1m commented Aug 9, 2025

cc @jgraham @allstarschh for Gecko, although Gecko already passes those tests.

@nt1m nt1m force-pushed the wpt-export-for-webkit-297161 branch 2 times, most recently from da3d4ed to 12a4a56 Compare August 9, 2025 06:36
@nt1m nt1m force-pushed the wpt-export-for-webkit-297161 branch from 12a4a56 to fa070ef Compare August 9, 2025 06:41
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the WebKit project.

@foolip
Copy link
Member

foolip commented Aug 14, 2025

@nt1m the change itself seems straightforward, but does it affect anything other than Modules? Is there any web compat risk, things that would previously work but now would not?

@nt1m
Copy link
Member Author

nt1m commented Aug 14, 2025

@nt1m the change itself seems straightforward, but does it affect anything other than Modules? Is there any web compat risk, things that would previously work but now would not?

The JSON change only affects modules and Web Inspector (not web-exposed) in WebKit (based on code-analysis). The XML change isn't covered by this WPT PR, but covers what gets detected as an XML document when loading one in-page and in XHR.

Gecko has shipped correct mimetype parsing for a while, and this change is limited to mimetypes with a suffix (e.g. +json and +xml) where we make sure every part is a valid token, so unless people use really funky mimetypes, I doubt there will be much breakage. Either way, this PR only adds the tests for JSON modules.

@foolip
Copy link
Member

foolip commented Aug 14, 2025

Thanks for the details @nt1m, sounds like it should be fine.

Microsoft is driving the Modules focus area of Interop 2025, so I'd like to hear from them too to avoid surprises. @dandclark can you take a look?

@dandclark
Copy link
Contributor

Sorry for the delay, we're looking at this. There's a question of how feasible/reasonable it is to limit this stricter checking to just +json MIME types, or if it should be applied more widely. Not sure yet about the compat impact of applying it out to all wildcard MIME type checks. I'll be back with our answer soon.

@dandclark
Copy link
Contributor

On further review this is fine from our side.

@nt1m
Copy link
Member Author

nt1m commented Aug 21, 2025

Thanks! will merge it

@nt1m nt1m merged commit d2adc80 into web-platform-tests:master Aug 21, 2025
26 checks passed
@nt1m nt1m deleted the wpt-export-for-webkit-297161 branch August 21, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants