Merge pull request #29127 from DmytroVasin/rails-ujs-remote-callbacks

Fix callback in rails ujs

3 files changed, 34 insertions, 69 deletions

diff --git a/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee b/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
index 852587042c..b3448dabac 100644
--- a/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
+++ b/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
@@ -62,7 +62,7 @@ Rails.handleRemote = (e) ->
         fire(element, 'ajax:send', [xhr])
       else
         fire(element, 'ajax:stopped')
-        xhr.abort()
+        return false
     success: (args...) -> fire(element, 'ajax:success', args)
     error: (args...) -> fire(element, 'ajax:error', args)
     complete: (args...) -> fire(element, 'ajax:complete', args)
diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee
index a653d3af3d..4d7848e162 100644
--- a/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee
+++ b/actionview/app/assets/javascripts/rails-ujs/utils/ajax.coffee
@@ -20,13 +20,12 @@ Rails.ajax = (options) ->
     else
       options.error?(response, xhr.statusText, xhr)
     options.complete?(xhr, xhr.statusText)
-  # Call beforeSend hook
-  options.beforeSend?(xhr, options)
-  # Send the request
+
+  unless options.beforeSend?(xhr, options)
+    return false
+
   if xhr.readyState is XMLHttpRequest.OPENED
     xhr.send(options.data)
-  else
-    fire(document, 'ajaxStop') # to be compatible with jQuery.ajax
 
 prepareOptions = (options) ->
   options.url = options.url or location.href
diff --git a/actionview/test/ujs/public/test/call-remote-callbacks.js b/actionview/test/ujs/public/test/call-remote-callbacks.js
index 707e21541d..48763f6301 100644
--- a/actionview/test/ujs/public/test/call-remote-callbacks.js
+++ b/actionview/test/ujs/public/test/call-remote-callbacks.js
@@ -12,34 +12,20 @@ module('call-remote-callbacks', {
     $(document).undelegate('form[data-remote]', 'ajax:send')
     $(document).undelegate('form[data-remote]', 'ajax:complete')
     $(document).undelegate('form[data-remote]', 'ajax:success')
-    $(document).unbind('ajaxStop')
     $(document).unbind('iframe:loading')
   }
 })
 
-function start_after_submit(form) {
-  form.bindNative('ajax:complete', function() {
-    ok(true, 'ajax:complete')
-    start()
-  })
-}
-
 function submit(fn) {
   var form = $('form')
-  start_after_submit(form)
 
   if (fn) fn(form)
   form.triggerNative('submit')
-}
 
-function submit_with_button(submit_button) {
-  var form = $('form')
-  start_after_submit(form)
-
-  submit_button.triggerNative('click')
+  setTimeout(function() { start() }, 13)
 }
 
-asyncTest('modifying form fields with "ajax:before" sends modified data in request', 4, function() {
+asyncTest('modifying form fields with "ajax:before" sends modified data in request', 3, function() {
   $('form[data-remote]')
     .append($('<input type="text" name="user_name" value="john">'))
     .append($('<input type="text" name="removed_user_name" value="john">'))
@@ -61,7 +47,7 @@ asyncTest('modifying form fields with "ajax:before" sends modified data in reque
   })
 })
 
-asyncTest('modifying data("type") with "ajax:before" requests new dataType in request', 2, function() {
+asyncTest('modifying data("type") with "ajax:before" requests new dataType in request', 1, function() {
   $('form[data-remote]').data('type', 'html')
     .bindNative('ajax:before', function() {
       this.setAttribute('data-type', 'xml')
@@ -74,7 +60,7 @@ asyncTest('modifying data("type") with "ajax:before" requests new dataType in re
   })
 })
 
-asyncTest('setting data("with-credentials",true) with "ajax:before" uses new setting in request', 2, function() {
+asyncTest('setting data("with-credentials",true) with "ajax:before" uses new setting in request', 1, function() {
   $('form[data-remote]').data('with-credentials', false)
     .bindNative('ajax:before', function() {
       this.setAttribute('data-with-credentials', true)
@@ -96,14 +82,11 @@ asyncTest('stopping the "ajax:beforeSend" event aborts the request', 1, function
     form.unbind('ajax:send').bindNative('ajax:send', function() {
       ok(false, 'ajax:send should not run')
     })
-    form.unbind('ajax:complete').bindNative('ajax:complete', function() {
-      ok(false, 'ajax:complete should not run')
-    })
-    form.bindNative('ajax:error', function(e, xhr, status, error) {
+    form.bindNative('ajax:error', function(e, response, status, xhr) {
       ok(false, 'ajax:error should not run')
     })
-    $(document).bindNative('ajaxStop', function() {
-      start()
+    form.bindNative('ajax:complete', function() {
+      ok(false, 'ajax:complete should not run')
     })
   })
 })
@@ -188,16 +171,13 @@ asyncTest('"ajax:beforeSend" can be observed and stopped with event delegation',
     form.unbind('ajax:send').bindNative('ajax:send', function() {
       ok(false, 'ajax:send should not run')
     })
-    form.unbind('ajax:complete').bindNative('ajax:complete', function() {
+    form.bindNative('ajax:complete', function() {
       ok(false, 'ajax:complete should not run')
     })
-    $(document).bindNative('ajaxStop', function() {
-      start()
-    })
   })
 })
 
-asyncTest('"ajax:beforeSend", "ajax:send", "ajax:success" and "ajax:complete" are triggered', 9, function() {
+asyncTest('"ajax:beforeSend", "ajax:send", "ajax:success" and "ajax:complete" are triggered', 8, function() {
   submit(function(form) {
     form.bindNative('ajax:beforeSend', function(e, xhr, settings) {
       ok(xhr.setRequestHeader, 'first argument to "ajax:beforeSend" should be an XHR object')
@@ -218,25 +198,25 @@ asyncTest('"ajax:beforeSend", "ajax:send", "ajax:success" and "ajax:complete" ar
   })
 })
 
-if(window.phantom !== undefined) {
-  asyncTest('"ajax:beforeSend", "ajax:send", "ajax:error" and "ajax:complete" are triggered on error', 7, function() {
-    submit(function(form) {
-      form.attr('action', '/error')
-      form.bindNative('ajax:beforeSend', function(arg) { ok(true, 'ajax:beforeSend') })
-      form.bindNative('ajax:send', function(arg) { ok(true, 'ajax:send') })
-      form.bindNative('ajax:error', function(e, xhr, status, error) {
-        ok(xhr.getResponseHeader, 'first argument to "ajax:error" should be an XHR object')
-        equal(status, 'error', 'second argument to ajax:error should be a status string')
-        // Firefox 8 returns "Forbidden " with trailing space
-        equal($.trim(error), 'Forbidden', 'third argument to ajax:error should be an HTTP status response')
-        // Opera returns "0" for HTTP code
-        equal(xhr.status, window.opera ? 0 : 403, 'status code should be 403')
-      })
+asyncTest('"ajax:beforeSend", "ajax:send", "ajax:error" and "ajax:complete" are triggered on error', 8, function() {
+  submit(function(form) {
+    form.attr('action', '/error')
+    form.bindNative('ajax:beforeSend', function(arg) { ok(true, 'ajax:beforeSend') })
+    form.bindNative('ajax:send', function(arg) { ok(true, 'ajax:send') })
+    form.bindNative('ajax:error', function(e, response, status, xhr) {
+      equal(response, '', 'first argument to ajax:error should be an HTTP status response')
+      equal(status, 'Forbidden', 'second argument to ajax:error should be a status string')
+      ok(xhr.getResponseHeader, 'third argument to "ajax:error" should be an XHR object')
+      // Opera returns "0" for HTTP code
+      equal(xhr.status, window.opera ? 0 : 403, 'status code should be 403')
+    })
+    form.bindNative('ajax:complete', function(e, xhr, status) {
+      ok(xhr.getResponseHeader, 'first argument to "ajax:complete" should be an XHR object')
+      equal(status, 'Forbidden', 'second argument to ajax:complete should be a status string')
     })
   })
-}
+})
 
-// IF THIS TEST IS FAILING, TRY INCREASING THE TIMEOUT AT THE BOTTOM TO > 100
 asyncTest('binding to ajax callbacks via .delegate() triggers handlers properly', 4, function() {
   $(document)
     .delegate('form[data-remote]', 'ajax:beforeSend', function() {
@@ -245,29 +225,15 @@ asyncTest('binding to ajax callbacks via .delegate() triggers handlers properly'
     .delegate('form[data-remote]', 'ajax:send', function() {
       ok(true, 'ajax:send handler is triggered')
     })
-    .delegate('form[data-remote]', 'ajax:complete', function() {
-      ok(true, 'ajax:complete handler is triggered')
-    })
     .delegate('form[data-remote]', 'ajax:success', function() {
       ok(true, 'ajax:success handler is triggered')
     })
-  $('form[data-remote]').triggerNative('submit')
-
-  setTimeout(function() {
-    start()
-  }, 63)
-})
-
-asyncTest('binding to ajax:send event to call jquery methods on ajax object', 2, function() {
-  $('form[data-remote]')
-    .bindNative('ajax:send', function(e, xhr) {
-      ok(true, 'event should fire')
-      equal(typeof(xhr.abort), 'function', 'event should pass jqXHR object')
-      xhr.abort()
+    .delegate('form[data-remote]', 'ajax:complete', function() {
+      ok(true, 'ajax:complete handler is triggered')
     })
-    .triggerNative('submit')
+  $('form[data-remote]').triggerNative('submit')
 
-  setTimeout(function() { start() }, 35)
+  setTimeout(function() { start() }, 13)
 })
 
 })()