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

Rails needs xhr.setRequestHeader("Accept", "text/javascript"); #52

Closed
gygelly opened this issue Nov 23, 2010 · 15 comments
Closed

Rails needs xhr.setRequestHeader("Accept", "text/javascript"); #52

gygelly opened this issue Nov 23, 2010 · 15 comments

Comments

@gygelly
Copy link

gygelly commented Nov 23, 2010

So I made a form the rails way, with:
form_tag, :remote=>true`

The controller it submitted to had the standard:

respond_to do |format|
    format.html 
    format.js
end

When I submitted the form, it did submit as an xhr request, but rails responded by rendering the .html.erb instead of the .js.rjs

This is because the Request Header: Accept is set to / (discovered via Chrome's resources tab).

To fix this, I had to edit your rails.js at line 56-59 the following way:

beforeSend: function (xhr) {
  xhr.setRequestHeader("Accept", "text/javascript");
  el.trigger('ajax:loading', xhr);
},

I'm not submitting this as a patch, as I suspect this needs to be added in other places, but I'm not nearly smart enough to know where.

@neerajsingh0101
Copy link

Just adding to the ticket that Safari, Chrome and Firefox send HTTP_ACCEPT as only

*/* 

when form is submitted. And yes the xhr? is true in all the three cases.

@neerajsingh0101
Copy link

Also note that webkit appends to the list rather than replacing it.

http://bugs.jquery.com/ticket/6230

However that is okay with Rails because rails puts text/javascript as higher priority.

> Mime::Type.parse('*/*, text/javascript')
 => [#<Mime::Type:0x10107a830 @symbol=:js, @string="text/javascript",     
@synonyms=["application/javascript", "application/x-javascript"]>, 
#<Mime::Type:0x102f7ec90 @symbol=nil, @string="*/*", @synonyms=[]>] 

@neerajsingh0101
Copy link

I personally believe that this problem should be fixed on the server side (with rails). However until that is done you can use a solution I use in all my projects. With this solution you don't need to edit rails.js. Also with this file your app will continue to work even if you are submitting an ajax request without using rails.js. .

https://github.com/neerajdotname/admin_data/blob/master/lib/js/advance_search/global_ajax_setting.js

@gygelly
Copy link
Author

gygelly commented Nov 23, 2010

neerajdotname, I'd love to use your code, but I can't get it to work.
Right now, my head is like this:

<script src="/javascripts/jquery-1.4.4.min.js"></script>
<script src="/javascripts/rails.js"></script> 
<script>
$(function() {

  $.ajaxSetup({
    'beforeSend': function(xhr) {
      xhr.setRequestHeader("Accept", "text/javascript");
    }
  });

});
</script> 

But the request header: accept is still /
Any tips?

@neerajsingh0101
Copy link

You are right. My solution did not work with rails.js .

Anyway I tested the scenario with modified rails.js and it worked. Here is rails.js version.

https://gist.github.com/712518

I changed line number #57 if you are curious. This should work. Let me know.

I will be putting in a proper fix in a few days with test and everything.

@gygelly
Copy link
Author

gygelly commented Nov 23, 2010

Yeah, if you look at my original post, that's pretty much how I fixed it in the first place.
The only problem with yours is that you removed the old line 57:
el.trigger('ajax:loading', xhr);
which I kept so that the loading callback still works.

@neerajsingh0101
Copy link

Ahh. I totally missed that. So your code is working.

I will investigate why ajax:loading callback did not work. And will also change the name from ajax:loading to ajax:beforeSend to be closer to jQuery. For a while I thought ajax:loading is fired after the remote call is invoked.

@gygelly
Copy link
Author

gygelly commented Nov 23, 2010

it is.
see, ajax:loading is called by beforeSend in rails.js at line 57 already. It works fine as long as you don't delete the current line 57.

@neerajsingh0101
Copy link

If an ajax request is being sent then
HTTP_ACCEPT must have text/javascript.

Closed by b6a3500

@mislav
Copy link
Member

mislav commented Dec 17, 2010

This just bit me; I blamed the line and finished here.

I must disagree with this change and this whole discussion. What you did here is just limited all responses from Rails to just javascript ones. If a Rails app wants to respond with a, say, HTML fragment that an "ajax:success" handler would append onto the page, this makes it impossible because the "Accept" header is hard locked to "text/javascript" and Rails template lookup will fail.

You didn't even make it possible to override this header in "ajax:beforeSend". In prototype-ujs, I've made the xhr object accessible at this stage for that exact purpose:

document.on('ajax:create', 'form.new_conversation', function(event) {
  var request = event.memo.request
  request.options.requestHeaders = {'Accept': 'text/html'}
})

Please, keep the Accept header at */*. This allows us to send any kind of data from the server back to the script, be it JS, HTML or XML. And as for our original poster's troubles, he could have configured Rails to prefer js format for */* requests by simply reordering formats:

respond_to do |format|
    format.js
    format.html 
end

Web browsers would still get served HTML.

Also, the xhr object needs to be accessible to users in the "beforeSend" phase.

If you agree, I can make the necessary changes (I'm a committer).

@neerajsingh0101
Copy link

I agree.

If I understand you correctly you patch will take care of two things

  1. set Accept header to /
  2. pass xhr object in ajax:beforeSend

Thanks

@krzkrzkrz
Copy link

I've been having a similar problem. I'm a bit confused with the way this current conversation is going. What is the final decision on this? Is there an updated version that we can use?

@neerajdotname, what patch are you referring to?

Why was line 57 removed at cff3fa9
I don't understand the benefit of having this line removed. I cannot override the header. I tried putting the following in application.js:

jQuery.ajaxSetup({
beforeSend: function (xhr) { xhr.setRequestHeader("Accept", "text/javascript")}
})

For your information. My issue in detail can be found here: http://stackoverflow.com/questions/4533743/ajax-comments-not-working-in-rails-3-0-3

@neerajsingh0101
Copy link

This commit fbbefa0 should satisfy everyone's need.

The current code by default doest not set any HTTP ACCEPT header since I put in this commit

cff3fa9

Now I am going to assume that you have following code

def destroy
  @user = User.find(params[:id])
  @user.destroy

  respond_to do |format|
    format.html { redirect_to(users_url) }
    format.xml  { head :ok }
    format.js   {          }
  end
end

Note that the index page has :remote => true

<%= link_to 'Destroy', user, :confirm => 'Are you sure?', :method => :delete, :remote => true %>

If you click on Destroy then browser will send

/
and format.html will be executed. Your goal is to execute format.js.

Add following lines in your javascript right after jquery is loaded.

jQuery.ajaxSetup({ beforeSend: function (xhr) { xhr.setRequestHeader("Accept", "text/javascript"); } });

Now format.js will be executed.

@krzkrzkrz
Copy link

Thanks neerajdotname. The newer version now works accordingly.

jQuery.ajaxSetup({ beforeSend: function (xhr) { xhr.setRequestHeader("Accept", "text/javascript"); } }); is now overriding appropriately.

Keep up the good work.

@neerajsingh0101
Copy link

@krzkrzkrz thanks for confirming that it is working. Sweet !!!

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

No branches or pull requests

4 participants