Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource overrideMimeType not working on Edge #7085

Closed
thw0rted opened this issue Sep 27, 2018 · 5 comments
Closed

Resource overrideMimeType not working on Edge #7085

thw0rted opened this issue Sep 27, 2018 · 5 comments

Comments

@thw0rted
Copy link
Contributor

TL;DR: I found a spec-compliance issue with how Resource does XHRs.

I came here to report an issue I was having with Edge, but to my surprise it isn't Edge's fault. I have some code that makes a Resource and calls fetchXML, treating the response as an XMLDocument. This failed in Edge, which led me to dig a bit -- I noticed that my server was returning the (definitely valid XML, not HTML) content with a Content-Type of text/html. I don't know why, it's not under my control, so I'm stuck with it. Firefox and Chrome return an XMLDocument anyway; Edge was returning an HTMLDocument. I thought this was the bug, until I did a bunch of extra testing.

I found out that the XHR spec was recently updated to state that calling open() must reset the "override MIME type" -- see bullet 12 in the current spec. I saw that the Resource class calls overrideMimeType before calling open. This results in the override getting ignored in recent builds of Edge, and the problem will get worse as other browsers come into compliance with the changes.

I believe the fix is as simple as moving the open call before the call to overrideMimeType. This would be a PR instead of an issue, but my CLA issues remain unresolved, so I'll just point 👉

@thw0rted
Copy link
Contributor Author

Update: WhatWG is changing their mind about this logic, so maybe the other browsers won't be matching Edge after all. Still, the fact remains that this breaks on some shipping Edge versions and I don't think it hurts to reorder the logic in Resource.

@hpinkos
Copy link
Contributor

hpinkos commented Sep 27, 2018

Hmm, I wonder if this is related so some other weirdness we've been seeing in Edge recently. Thanks for bring this up @thw0rted!

@mramato @tfili what are your thoughts on this?

@mramato
Copy link
Contributor

mramato commented Sep 27, 2018

I'm not sure this was the same thing I was seeing, but couldn't hurt. Thanks for the write up @thw0rted, I'll open a PR just to cover our bases.

mramato added a commit that referenced this issue Sep 27, 2018
As per the updated XHR spec, you need to call open before calling
`overrideMimeType`.  While they are most likely rolling back this portion
of the spec, it is currently required in Edge and is harmless to keep
this pay even after the spec is reverted.

Fixes #7085
@mramato
Copy link
Contributor

mramato commented Sep 27, 2018

Thanks @thw0rted! I was able to replicate this behavior and opened #7091 with the fix.

@hpinkos
Copy link
Contributor

hpinkos commented Sep 28, 2018

Thanks again for reporting this @thw0rted! We just merged a fix for this into master and it will be included in the 1.50 release on Monday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants