This project is archived and is in readonly mode.

#5975 ✓resolved
Christiaan Van den Poel

put :update in json format should return '{}' in real-life and not only in tests

Reported by Christiaan Van den Poel | November 15th, 2010 @ 01:51 PM

There is a bug in jQuery 1.4.4 which causes an succesful ajax update to a resource to trigger 'ajax:failure' events instead of 'ajax:success'.

The reason for this is, that the server answers with status 200 and a nearly empty body, it contains 1 whitespace. This causes jQuery to choke on the parseJSON. And so the wrong ajax callbacks are triggered.

This one is easy to reproduce:

 #generate a new rails project
 $ rails new ujs_test && cd ujs_test
 # add the line 'gem 'jquery-rails' to your Gemfile
 $ bundle install
 $ rails g jquery:install
 $ rails g scaffold folder name:string

In the 'app/views/folders/_form.html.erb' change the first line to:

<%= form_for(@folder, :remote => true, :html => {'data-type' => 'json' }) do |f| %>

Be sure to use just responders:

class FoldersController < ApplicationController
respond_to :html, :json

def index
@folders = Folder.all
respond_with(@folders)
end

def edit
@folder = Folder.find(params[:id])
respond_with(@folder)
end

def new
@folder = Folder.new
respond_with(@folder)
end

def create
@folder = Folder.create(params[:folder])
respond_with(@folder)
end

def update
@folder = Folder.find(params[:id]).update_attributes(params[:folder])
respond_with(@folder)
end
end

As a debug/logger you can use this in your application.js

  // Place your application-specific JavaScript functions and classes here
  // This file is automatically included by javascript_include_tag :defaults
  $(function(){
      $('form[data-remote]').bind('ajax:success', function(data, status, xhr){
          console.log("*** ajax:success ***");
          console.dir(data);
          console.dir(status);
          console.log("*** END: ajax:success ***");
      });

      $('form[data-remote]').bind('ajax:failure', function(xhr, status, error){
          console.log("+++ ajax:failure +++");
          console.dir(xhr);
          console.dir(status);
          console.dir(error);
          console.log("+++ END: ajax:failure +++");
      });
  });

When you go to http://localhost/folders/1/edit (besure to have one created already).

Doing an edit and hitting update will trigger an 'ajax:failure' event although the response status is 200.

Comments and changes to this ticket

  • Neeraj Singh

    Neeraj Singh November 15th, 2010 @ 02:10 PM

    • Assigned user set to “Neeraj Singh”
    • Importance changed from “” to “Low”

    I'll take a look.

  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 02:11 PM

    • Assigned user cleared.

    The bug was rejected by the jquery team http://bugs.jquery.com/ticket/7521

    They say the string with whitespaces is not valid json, so it's in the rails code where the whitespaces should be removed from the response, no?

  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 02:24 PM

    In the file 'actionpack/lib/action_controller/metal/responder.rb' on line 167
    https://github.com/rails/rails/blob/master/actionpack/lib/action_co...

    The code is that for a successful update the response is

    head :ok
    

    In the file 'actionpack/lib/action_controller/metal/head.rb' on line 29
    https://github.com/rails/rails/blob/master/actionpack/lib/action_co...

    There is code that set the response body to a string with one whitespace, which is, according the jQuery team non valid when you return json.

     self.response_body = " "
    

    I can imagine there is a historic reason to return one whitespace in case of a head response, but I also follow the jQuery team when they state that in case of a JSON response this string/response is invalid.

  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 02:25 PM

    • Assigned user set to “Neeraj Singh”

    Seems during my comment edit I've removed the who's responsible?

  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 03:42 PM

    When I look at the rails code there is a test for returning '{}' when a put request is made in json format.

    https://github.com/rails/rails/blob/master/actionpack/test/controll...

    But when I use it in my application and browser, I get " " which is invalid json.

    The thing is also: the accepts request header is:

    Chrome 7: application/json, text/javascript, /; q=0.01
    FF3: application/json, text/javascript, /; q=0.01

    So when I write a new test:

    def test_using_resource_for_put_with_json_yields_ok_on_success_2
       Customer.any_instance.stubs(:to_json).returns('{"name": "David"}')
       @request.accept = "application/json,text/javascript,*/*;q=0.01"
       put :using_resource
       assert_equal "application/json", @response.content_type
       assert_equal 200, @response.status
       assert_equal "{}", @response.body
    end
    

    this fails (exception):

    NoMethodError: undefined method `hash_for_customer_url' for #<Module:0xb6936fb4>
    /var/projects/github/rails@github/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:135:in `__send__'
    /var/projects/github/rails@github/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:135:in `polymorphic_url'
    /var/projects/github/rails@github/actionpack/lib/action_dispatch/routing/url_for.rb:138:in `url_for'
    /var/projects/github/rails@github/actionpack/lib/action_controller/metal/redirecting.rb:93:in `_compute_redirect_to_location'
    /var/projects/github/rails@github/actionpack/lib/action_controller/metal/redirecting.rb:63:in `redirect_to'
    ....
    

    So then I write the test like this:

         def test_using_resource_for_put_with_json_yields_ok_on_success_2
        with_test_route_set do
        Customer.any_instance.stubs(:to_json).returns('{"name": "David"}')
            @request.accept = "application/json,text/javascript,*/*;q=0.01"
            put :using_resource
            assert_equal "application/json", @response.content_type
            assert_equal 200, @response.status
            assert_equal "{}", @response.body
        end
      end
    

    The test fails (real test fail):

    1) Failure:
    test_using_resource_for_put_with_json_yields_ok_on_success_2(RespondWithControllerTest)
    [controller/mime_responds_test.rb:742:in `test_using_resource_for_put_with_json_yields_ok_on_success_2'
     controller/mime_responds_test.rb:932:in `with_test_route_set'
     /var/projects/github/rails@github/actionpack/lib/action_dispatch/testing/assertions/routing.rb:164:in `with_routing'
     controller/mime_responds_test.rb:924:in `with_test_route_set'
     controller/mime_responds_test.rb:738:in `test_using_resource_for_put_with_json_yields_ok_on_success_2'
     /var/projects/github/rails@github/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `__send__'
     /var/projects/github/rails@github/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `run'
     /var/projects/github/rails@github/activesupport/lib/active_support/callbacks.rb:428:in `_run_setup_callbacks'
     /var/projects/github/rails@github/activesupport/lib/active_support/testing/setup_and_teardown.rb:65:in `run']:
     <"application/json"> expected but was
     <"text/html">.
    

    So I'm testing with a reallife situation and the test fails?

    Perhaps I'm not getting all this rails code and am I doing it wrong, perhaps someone can help out here.

  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 03:49 PM

    • Title changed from “latest jquery (1.4.4) has a bug in its parseJSON that has an effect on jquery-ujs and its callbacks” to “put :update in json format should return '{}' in real-life and not only in tests”
  • Neeraj Singh

    Neeraj Singh November 15th, 2010 @ 05:16 PM

    I followed your instruction and I am getting {} as the response.

  • Neeraj Singh
  • Christiaan Van den Poel

    Christiaan Van den Poel November 15th, 2010 @ 05:39 PM

    indeed, it look likes it is fixed in edge since 3.0.1.

    My mistake...

    Thx for checking

  • Neeraj Singh

    Neeraj Singh November 19th, 2010 @ 04:45 PM

    • State changed from “new” to “resolved”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages