STR: open any classified ad on the website leboncoin.fr (eg http://www.leboncoin.fr/velos/861403185.htm?ca=5_s ) and click on the orange geolocation icon to display the map in the middle of the page. Result: only black background overlay, no map. Reg range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3c3a8eed0578&tochange=c72d38e7a212 Boris Zbarsky — Bug 814050. XMLHttpRequest.withCredentials header should throw on main thread too, in some cases. r=peterv I think it's a TE bug. What do you think, Boris?
Are they setting withCredentials on a sync XHR and expecting that to not throw? If so, TE indeed, though note that other browsers do seem to allow this (albeit with deprecation messages to the console), so if the site won't budge we'd just have to give up on this and get the spec changed, probably....
Error in the console after clicking on the icon: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help http://xhr.spec.whatwg.org/ http://www.leboncoin.fr/js/beta_full_54266.js:1:5195 Use of XMLHttpRequest's withCredentials attribute is no longer supported in the synchronous mode in window context. http://www.leboncoin.fr/js/beta_full_54266.js:1:5353 LeBoinCoin is pretty popular, it's the 1st French free classified ad website.
OK. I mean, we're doing what the spec says to do. I just tested Chrome with your steps, and the console says: Setting 'XMLHttpRequest.withCredentials' for synchronous requests is deprecated. but that doesn't throw in Chrome (yet); see comment 1. If we think the spec should change, I guess we should bring this up with the spec editor.... And probably with the Chrome folks, since their refusal to implement the spec is what's getting us here. :(
Happy to change the specification here if we decide TE doesn't work: https://github.com/whatwg/xhr/issues/new Philip, any idea why Chrome has never disabled this despite the specification requiring it?
Let's see, it was deprecated in June 2014: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/BOgLhbdljU4 It looks like Mahesh moved on to other things and nobody else noticed or took ownership of this. Usage as fallen quite a bit but is still not a rounding error: https://www.chromestatus.com/metrics/feature/timeline/popularity/472 Before I try to find a new owner for this or try to change it myself, why does it make sense to throw here, is sync XHR extra problematic with credentials?
We disabled it as part of an effort to deprecate synchronous XMLHttpRequest.
OK, so in principle you could have given timeout and responseType the same treatment, but you wanted to start with something you suspect has lower usage? Not sure what to think about this, it goes back to https://github.com/whatwg/xhr/issues/20 where I agree that getting rid of sync XHR sure would be nice, but how to actually do it I have no idea. Is there reason to suspect that throwing here would have a leveraged effect, so that sync XHR usage would drop by more than the usage of sync XHR with credentials?
I think we started with this just after this feature was implemented. timeout and responseType existed for much longer. I think if we offered these features synchronous XMLHttpRequest usage might increase, since there's less motivation to use asynchronous networking.
In Gecko, is this restriction artificial in the sense that it could easily be lifted? It would be artificial in Blink, and I'm really leaning towards suggesting that the spec should just allow this with sync XHR. It's the only bit of the API with this restriction, and new things are going into Fetch anyway. The usage (~0.02% vs ~1.4%) means that it would make a very small dent in sync XHR usage, at the cost of issues like this. Sad face :(
> In Gecko, is this restriction artificial in the sense that it could easily be lifted? Yes.
OK, so I propose to drop this restriction from spec and implementations. There's currently a deprecation message in Blink that would go away as well. Would that be acceptable, or does anyone see this as important to the overall sync XHR effort, and if so why?
Olli, thoughts on comment 11?
I can live with dropping this particular restriction. Philip, is this the only sync-XHR restriction not supported by Blink?
The short answer is that no sync XHR restrictions are supported by Blink in that nothing throws an exception. There's one other thing that's deprecated (but doesn't throw an exception) and that's in open(), and only on the main thread if we're not currently dispatching the beforeunload event. It's this use counter: https://www.chromestatus.com/metrics/feature/timeline/popularity/581 I now see that the timeout and responseType setters should also throw if the sync flag is set, which I just assumed wasn't the case when writing comment #7, do these also throw in Gecko now? Whatever the case may be, I think that only showing deprecation messages or throwing in open() would make sense, at least unless there's a reason to think that deprecating certain combinations together with XHR would have a leveraged effect in driving down usage.
The idea was to start throwing on all the "new features" when used with sync XHR, so that it wasn't just possible to use them. Apparently blink just failed pretty well with following the spec here. .responseType setter does throw in Gecko when used with sync XHR in main thread, and so does .timeout setter. And open() does throw too.
Gah, again I lie because I grep for the wrong thing (deprecations instead of exceptions). Blink does throw in the timeout and responseType, and for the corresponding checks in open(). Only the withCredentials bits seems to be missing. Looks like the last time anyone looked at the exceptions in WebKit, the spec didn't require this yet: https://trac.webkit.org/changeset/43459 https://web.archive.org/web/20090428125543/http://dev.w3.org/2006/webapi/XMLHttpRequest-2 Given that Gecko already does this and that it's part of a group of things, I'm more uncertain about what to do. Has anyone checked the behavior of IE11 or Edge?
Edge works but puts a warning in the console: "SCRIPT7015: Setting withCredentials attribute for synchronous XMLHttpRequest is deprecated" IE11 works and puts no warning in the console. What do you think, Boris?
I think it's worth trying to keep this if we think Blink and Edge will follow very soon. Otherwise, I think we should give up, and just change the spec.
I think we should give up for the withCredentials case FWIW. Happy to change the standard.
Sync XHR withCredentials is in the XHR spec now: https://github.com/whatwg/xhr/commit/90d7058031aca487f4480be07dc26cefbc4103c4 Boris, do you have time to fix this?
Created attachment 8748835 [details] [diff] [review] Start allowing sync XHR with credentials in window scopes again, since no one except for us bothered to implement the spec Tests still running, so there may be more test-only changes, I guess
2 years ago
Comment on attachment 8748835 [details] [diff] [review] Start allowing sync XHR with credentials in window scopes again, since no one except for us bothered to implement the spec oh, lovely, wpt tests required old behavior, but other browsers haven't cared of fixing their implementations. Though, I guess that happens rather easily.
Actually, wpt tests were inconsistent. Some required the old behavior, some require the new one. We still fail the latter ones because we throw the wrong exception later in the test, which is why it doesn't show up in the diff. It's all quite lovely.