Bug Tracker

Ticket #13240 (closed bug: fixed)

Opened 22 months ago

Last modified 9 months ago

IE8 Doesn't support HTTP PATCH method

Reported by: pinciotti.dan182@… Owned by:
Priority: high Milestone: 1.11
Component: ajax Version: git
Keywords: Cc: jaubourg
Blocking: Blocked by:

Description

When trying to make an AJAX call using the 'PATCH' HTTP method, IE8 throws an exception. The exception is simply "Invalid Argument", and is thrown as soon as the following code is ran:

xhr.open('patch', /item/1, true);

It looks like the Microsoft XMLHTTP ActiveXObject does work with PATCH, so I was able to fix my problem by changing the order in which the XHR object is created so that the ActiveXObject is tried first.

A simple way to test this is to open up IE8 and try to call

$.ajax({
  type: "PATCH",
  url: "some.php",
  data: { name: "John", location: "Boston" }
});

in IE8. The browser won't make the request.

Change History

comment:1 Changed 22 months ago by anonymous

JSFiddle for the issue here:  http://jsfiddle.net/K4GQa/ Works in IE9, FF, Chrome, etc...but in IE8 it fails.

comment:2 Changed 21 months ago by dmethvin

  • Status changed from new to closed
  • Resolution set to wontfix

This is too much of a rare corner case for us to deal with. As a workaround you can override the xhr factory in your ajax options for this one situation and still use $.ajax.

comment:3 Changed 20 months ago by jashkenas@…

Whoa there guys -- this isn't a rare corner case, and it really should be dealt with in jQuery. If the PATCH method is to be used at all, it needs to work even if your customer is using IE8 to browse your website. Failing that, PATCH should be forbidden entirely in jQuery.ajax, so that folks don't try to use it, and assume that it will work because it happens to in their development browser.

Fortunately, the fix should be easy enough, if the method is PATCH, and the browser is LTE IE8, use the ol' ActiveXHTTP object, as you would in other old IEs.

comment:4 Changed 20 months ago by luke@…

I would echo @jashkenas' comment. Interest in PATCH has been gaining steam the last few years, and people will try to use it. jQuery should IMO have a specified behavior: supported, unsupported, or supported behind a flag.

comment:5 Changed 20 months ago by markelog

  • Cc jabourg added

For IE7-8 only  these methods are allowed in native xhr, therefore this problem is a general one, i.e. this – xhr.open('connect', /item/1, true); and this xhr.open('never-existed', /item/1, true); will fail too.

Fortunately, the fix should be easy enough, if the method is PATCH, and the browser is LTE IE8, use the ol' ActiveXHTTP object, as you would in other old IEs.

We don't do UA-sniffing and i hope we never will, but this bug could be exposed through feature detection and fixed through whitelist of allowed HTTP-methods, without always succumbing to activeX way in IE7-8. But, again, if this is a rare case i don't know if we should.

@jabourg?

comment:6 Changed 20 months ago by dmethvin

Seems pretty rare to me. The user or a plugin writer can fix it by changing xhr in $.ajaxSetup() or in the $.ajax() call itself if they know they'll encounter the issue. Due to PTSD I have forgotten the other important edge-case differences between the native XMLHttpRequest object and the ActiveX one in IE7/8, but it seems like a bad idea to switch IE8 to ActiveX full time.

comment:7 Changed 20 months ago by jashkenas

It's rare at the moment because PATCH hasn't been started to be used widely yet ... now that new versions of Rails are starting to support it by default, and libraries like Backbone and Ember want to follow suit as soon as possible, it will become extremely common. Basically any situation that used to use a (technically-invalid) partial PUT via Ajax may desire to use PATCH going forwards.

I'm not suggesting that jQuery on IE8 go ActiveX full time ... just if you're sending a non-whitelisted method over XHR. It also has the benefit of fixing jQuery.ajax for other (perhaps future) HTTP methods on IE8.

In the end, having end-users of jQuery version- or feature- detect IE8 to effectively monkey-patch jQuery.ajax seems like the exact opposite in spirit to the usual reason why we rely on jQuery in the first place -- because its core methods can be relied on to support the same features cross-browser. If you leave this wontfix, you're telling a whole lotta different downstream libraries to all implement the same monkey-patch in the same way for the same situation.

Sorry for beating on my drum (I hate it when people do it to me) -- but I'm really eager to land PATCH support, and proper partial data updates, and want to do it right.

comment:8 Changed 20 months ago by dmethvin

This can be fixed outside jQuery. Write up your fix and start using it. Let's see how big it is and what else it breaks. That way you also don't need to tell everyone to upgrade to jQuery 1.11 or wherever it lands.

comment:9 Changed 20 months ago by dmethvin

  • Cc jaubourg added; jabourg removed
  • Status changed from closed to reopened
  • Resolution wontfix deleted

I saw you mentioned monkey patching, none of that is required. Just add xhr: function(){ return new window.ActiveXObject("Microsoft.XMLHTTP"); } to the list of $.ajax arguments when appropriate.

comment:10 Changed 20 months ago by dmethvin

  • Status changed from reopened to closed
  • Resolution set to patchwelcome

Moving this to patchwelcome pending the outcome of your experiment.

comment:11 Changed 20 months ago by jaubourg

You have two solutions to fix this in your code:

Change the xhr creation code globally (always use ActiveX if present):

if ( window.ActiveXObject ) {
  $.ajaxSetup({
    xhr: function() {
      return new window.ActiveXObject("Microsoft.XMLHTTP");
    }
  });
}

A more subtle approach is to create a prefilter for this that changes the xhr creation code dynamically given the type:

if ( window.ActiveXObject ) {
  $.ajaxPrefilter(function( options ) {
    if ( /^patch$/i.test( options.type ) ) {
      options.xhr = function() {
        return new window.ActiveXObject("Microsoft.XMLHTTP");
      };
    }
  });
}

The ajax architecture has been revamped/reworked in 1.5 for this kind of situations. It is now very easy to fix "temporary" problems without having to wait for a full version of jQuery and, even, without the need for jQuery core to handle the problem.

By temporary, I mean peculiarities in older but still supported platforms when using "new stuff".

comment:12 Changed 20 months ago by jashkenas

Alright -- I'll play along. Here's the (uh, ahem) patch that I'm using:

 https://github.com/documentcloud/backbone/commit/f495c3615b5484ed7647cfc4f4ad3e276327e136

... and here's the ticket, so you can follow along with the results:

 https://github.com/documentcloud/backbone/issues/2152

comment:13 Changed 20 months ago by jdalton

The check for old IE in @jashkenas' patch is insufficient as it will be used even if ActiveX is disabled and will be used in IE9+.

comment:14 Changed 9 months ago by markelog

  • Status changed from closed to reopened
  • Resolution patchwelcome deleted

comment:15 Changed 9 months ago by markelog

  • Status changed from reopened to closed
  • Resolution set to fixed
  • Milestone changed from None to 1.11

comment:16 Changed 9 months ago by dmethvin

  • Priority changed from undecided to high
  • Component changed from unfiled to ajax
Note: See TracTickets for help on using tickets.