Fix server-generated JS response processing on IE9 #29108
Conversation
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @kaspth (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
| response = processResponse(xhr.response, xhr.getResponseHeader('Content-Type')) | ||
| xhrResponseObject = xhr.response | ||
| xhrResponseObject = xhr.responseText if !xhrResponseObject? and typeof xhr.responseText == 'string' # for IE9 | ||
| response = processResponse(xhrResponseObject, xhr.getResponseHeader('Content-Type')) |
There was a problem hiding this comment.
Think you can just do:
response = processResponse(xhr.response ? xhr.responseText, xhr.getResponseHeader('Content-Type'))There was a problem hiding this comment.
If you're happy to drop the type check, sure. That came from jQuery and appeared to be a workaround of other browser nasties, although I didn't dig too deep. Done.
…js and remote: true
|
Thanks @inopinatus! 💪 @javan appreciate the help with the JS review too 😊 |
Summary
Fix for #29069, IE9's lack of
xhr.response. With this in place, server-generated JS responses, Turbolinks redirects etc should now be executed by IE9 when given in response to a rails-ujsremote: truerequest.Other Information
This PR is not intended to make the rails-ujs test suite pass in IE9. That might be desirable, but it's more work than I could justify this week to nurse a dying browser. Tested with https://github.com/inopinatus/ujs-test-app instead.
xhr.response || xhr.responseTextin the call toprocessResponse? Because falsy values other thannullare still valid response objects.xhrResponseObject = if xhr.response? then xhr.response else xhr.responseText? I felt this way showed the intent more clearly.