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

strip fragment from URL before sending request for IE<=10, which brea… #252

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
3 participants
@adros
Copy link
Contributor

adros commented Jan 27, 2017

IE9 & 10 do not strip fragment when it is provided in URL and send it to server.
(maybe also lower versions, I have tested those versions; IE11 is OK).

This used to be spec violation (#9 in https://www.w3.org/TR/2012/WD-XMLHttpRequest-20121206/#dom-xmlhttprequest-open), but for some reason it does not appear in spec any more (https://xhr.spec.whatwg.org/#the-open()-method). But anyway, I consider this behavior as incorrect.

I have reported this to MS as well (https://connect.microsoft.com/IE/feedback/details/785674/xmlhttprequest-sends-fragment-to-server), but without any success even after providing the test case.

@jsf-clabot

This comment has been minimized.

Copy link

jsf-clabot commented Jan 27, 2017

CLA assistant check
All committers have signed the CLA.

@dylans
Copy link
Member

dylans left a comment

Could you please add a test in https://github.com/dojo/dojo/blob/master/tests/unit/request/xhr.js so we can verify this with our automated tests? Perhaps under the .get section of that file?

@dylans dylans added this to the 1.12.2 milestone Jan 27, 2017

@dylans

This comment has been minimized.

Copy link
Member

dylans commented Feb 9, 2017

@adros, please see my comment about adding a test for this. Once that's done I'm happy to land this, thanks!

@dylans dylans modified the milestones: 1.12.3, 1.12.2 Feb 24, 2017

@adros

This comment has been minimized.

Copy link
Contributor Author

adros commented Mar 10, 2017

Sorry for delay,
I have just added the test. It fails in IE without the fix in commit 0a88f0d.
(tested in IE10)

I had to modify also xhr.service.js, which now returns in payload also request url.

@dylans

dylans approved these changes Mar 13, 2017

@dylans

This comment has been minimized.

Copy link
Member

dylans commented Apr 13, 2017

Thanks @adros , closed via 762bbd7 (master) and backported to fad15c1 (1.12) and c5c8fa5 (1.11). If we need this fix in 1.10 or earlier, please raise a separate PR against 1.10 without the Intern test.

@dylans dylans closed this Apr 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.