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

HTTP PATCH method not supported in IE8 #2152

Closed
ThePromoter opened this issue Jan 17, 2013 · 15 comments
Closed

HTTP PATCH method not supported in IE8 #2152

ThePromoter opened this issue Jan 17, 2013 · 15 comments

Comments

@ThePromoter
Copy link

After reading the update notes I was really excited to implement the new patch functionality. I got it working perfectly, and then switched over to IE8 just to be sure it worked. Turns out IE8 does not recognize "PATCH" as a valid HTTP method, below is a list of supported methods:

The object permits only the following HTTP methods: "GET", "POST", "HEAD", "PUT", "DELETE", "MOVE", "PROPFIND", "PROPPATCH", "MKCOL", "COPY", "LOCK", "UNLOCK", "OPTIONS".

Is there a possible workaround to this?

EDIT: After doing a little more research, turns out this is certainly on jQuery's end. The ActiveXObject for XMLHTTP requests does support PATCH, but the native XMLHttpRequest in IE8 does not. jQuery ends up using the native version, since it's available to the browser. This is all in IE9, but using IE8 compatibility mode, I wonder if this issue would persist if the user was using native IE8. I'll try to test a little more and respond, but for now I'll try to submit a bug report to the jQuery team, since the problem is on their end.

@philfreo
Copy link
Contributor

If you wanted to replace PATCH with PUT you could use:

    // Override Backbone.sync to use the PUT HTTP method for PATCH requests
    // when doing Model#save({...}, { patch: true });
    var originalSync = Backbone.sync;
    Backbone.sync = function(method, model, options) {
        if (method === 'patch') options.type = 'PUT';
        return originalSync(method, model, options);
    };

@ThePromoter
Copy link
Author

This is great, thanks for posting that workaround, should keep things clean and if we ever decide to drop IE8 support we can simply remove this. This issue can be closed now as the only fix would be for jQuery's ajax method to prioritize the ActiveXHTTP object over the native XMLHTTPRequest object if available. I have no idea what other ramifications this would have, and an application could be rendered unable to save if the user disabled ActiveX, which is a lot more common than disabling JS.

Long story short, this is only really fixable by dropping IE8 support, heh.

@jashkenas
Copy link
Owner

Whoa there -- this sounds like it could be a bad bug ... and maybe something that should force us to reconsider supporting PATCH in the first place. Reopening.

@jashkenas jashkenas reopened this Jan 18, 2013
@philfreo
Copy link
Contributor

jQuery bug: http://bugs.jquery.com/ticket/13240

@cesutherland
Copy link

I feel supporting PATCH in Backbone is useful, but I am bumping into this issue too. The jQuery bug has been closed 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.

Rewriting PATCH to PUT by default (or providing configuration to do so) could be dangerous. Allowing PATCH to be broken in IE8 would cause a bug, but not a dangerous bug. It will obviously fail (vs. silently destroying data, which it could with PUT).

Another workaround is emulateHTTP. To do this just for PATCH:

// Override Backbone.sync to use X-HTTP-Method-Override: PATCH
// when doing Model#save({...}, { patch: true });
var originalSync = Backbone.sync;
Backbone.sync = function(method, model, options) {
  if (method === 'patch') {
    options.emulateHTTP = true;
  }
  return originalSync(method, model, options);
};

We now have 3 workarounds:

  1. Use PUT; implement API supporting PUT of partial set of attributes
  2. Override xhr factory to use ActiveX
  3. Use X-HTTP-Method-Override; implement API to support this, as described above.

If jQuery considers PATCH being broken in IE8 too much an edge case, maybe it is acceptable here too?

@jashkenas
Copy link
Owner

Grrr. This means we can't use PATCH. Partial PUT isn't acceptable, we shouldn't be second-guessing the jQuery XHR factory, and we can't assume that emulateHTTP will be supported by all back-ends. I'll take it out, and we can put it back in as soon as jQuery changes the wontfix to fixed.

@nsb
Copy link
Contributor

nsb commented Mar 19, 2013

It's really useful for backends that support emulateHTTP. We are able to use PATCH with emulateHTTP set only for IE8. Would be really sad to see it go. We have to use partial updates.

# if browser is IE8 use fake PATCH HTTP method
# we need this because IE8 doesn't support the PATCH method
if $.browser.msie and parseInt($.browser.version, 10) is 8
  originalSync = Backbone.sync
  Backbone.sync = (method, model, options) ->
    if (method is 'patch')
      options.emulateHTTP = true
    return originalSync(method, model, options)

@jashkenas
Copy link
Owner

@nsb I totally agree, and I really really want to include it. But we can't add a new HTTP method that only works if you configure your server to fake/tunnel faux HTTP methods.

@nsb
Copy link
Contributor

nsb commented Mar 19, 2013

The way I see it, it's optional to use it. What is the harm in leaving it in? I'd prefer to add a notifaction that PATCH is not supported by IE8. How would you do partial updates otherwise?

@jashkenas
Copy link
Owner

Backbone isn't going to include features that are going to prevent your app from working on particular (common) browsers -- period. If you want to do it yourself, feel free. $.ajax + model.set are always available.

@nsb
Copy link
Contributor

nsb commented Mar 19, 2013

k, I see you already added a comment in the jQuery bug. Hopefully they will pick up on that. I agree that is the best solution.

@jashkenas
Copy link
Owner

Alright -- following instructions from the jQuery team, I've added their suggested workaround. If you have a minute, please give master a try with your app, with PATCH, and with IE8 and IE7, and let me know how well it works for you.

@nsb
Copy link
Contributor

nsb commented Mar 20, 2013

I upgraded to Backbone master and removed my sync override for patch.
Works perfect in IE8. Our app doesn't run in IE7 so can't test that one. Looking good from here. Thanks.

@cesutherland
Copy link

This looks good to me! I have tested in IE7 and IE8.

This gives a 4 option to support this in an existing app on 9.10, again by over-riding sync. The following should work in general:

  var 
    originalSync = Backbone.sync;
  Backbone.sync = function (method, model, options) {
    options = options || {}; 
    if (!options.xhr && (method === 'patch' || options.type === 'PATCH') && window.ActiveXObject) {
      options.xhr = function() {
        return new ActiveXObject("Microsoft.XMLHTTP");
      };  
    }   
    return originalSync(method, model, options);
  };

@christiaanwesterbeek
Copy link

It may also occur in IE9 if your html has given IE reasons to believe it should operate in a lower version. To prevent this, do include IE=edge in the html header.

This forces IE9 to run in IE9 mode. Resource: http://stackoverflow.com/questions/5374099/how-do-i-force-internet-explorer-to-render-in-standards-mode-and-not-in-quirks

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

No branches or pull requests

6 participants