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

Preserve URL hash when requesting via $.ajax #1732

Closed
mgol opened this Issue Oct 20, 2014 · 14 comments

Comments

Projects
None yet
4 participants
@mgol
Member

mgol commented Oct 20, 2014

Originally reported by pferreir at: http://bugs.jquery.com/ticket/10495

While trying to inject the Facebook "like button" JS in a page in execution time, I got some funny behavior:

var fjs = $('script:first');
$('<script/>').attr({'async': true, 'src': '//connect.facebook.net/en_US/all.js#xfbml=1&appId=xxx'}).insertBefore(fjs)

fails (same with the equivalent before() call).

While

var fjs = $('script:first').get(0);
var e = $('<script/>').attr({'async': true, 'src': '//connect.facebook.net/en_US/all.js#xfbml=1&appId=xxx'}).get(0);
fjs.parentNode.insertBefore(e, fjs)

works just ok.

I tried different combinations of native/jQuery elements and they all fail. This happens at least since 1.6.1.

Issue reported for jQuery git

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: pferreir

I forgot to add that in the first case the element seems to be added to the DOM only to be immediately removed.

Member

mgol commented Oct 20, 2014

Comment author: pferreir

I forgot to add that in the first case the element seems to be added to the DOM only to be immediately removed.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: dmethvin

Why not use $.getScript ?  http://api.jquery.com/jQuery.getScript/

Can you provide a complete test case on jsfiddle.net? That would help us evaluate the problem.

Member

mgol commented Oct 20, 2014

Comment author: dmethvin

Why not use $.getScript ?  http://api.jquery.com/jQuery.getScript/

Can you provide a complete test case on jsfiddle.net? That would help us evaluate the problem.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: pferreir

Hello,

Here is a small example:  http://jsfiddle.net/RQVsa/

The uncommented code should work (show the "Like" button), while the commented on should fail. I tried $.getScript, with no success...

Member

mgol commented Oct 20, 2014

Comment author: pferreir

Hello,

Here is a small example:  http://jsfiddle.net/RQVsa/

The uncommented code should work (show the "Like" button), while the commented on should fail. I tried $.getScript, with no success...

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: timmywil

We filter out scripts in manipulation. getScript should be used, but we remove hashes from the url. I'm not sure why, but it's been there a while.

Member

mgol commented Oct 20, 2014

Comment author: timmywil

We filter out scripts in manipulation. getScript should be used, but we remove hashes from the url. I'm not sure why, but it's been there a while.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: dmethvin

Seems valid enough to keep open, until we figure out why we're removing the hash.

Member

mgol commented Oct 20, 2014

Comment author: dmethvin

Seems valid enough to keep open, until we figure out why we're removing the hash.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: dmethvin

Okay, this gives me an error from Facebook because of the xxx id:

 http://jsfiddle.net/RQVsa/10/

This gives the same error and doesn't have the hash as timmywil said:

 http://jsfiddle.net/RQVsa/11/

Since the script hasn't yet been executed the .insertBefore() is making the ajax request to request the script, but since the hash is stripped it's never going to see the correct id.

@jaubourg, do you know why we're stripping off the hash? Is there a specific reason for it, or just an omission?

Member

mgol commented Oct 20, 2014

Comment author: dmethvin

Okay, this gives me an error from Facebook because of the xxx id:

 http://jsfiddle.net/RQVsa/10/

This gives the same error and doesn't have the hash as timmywil said:

 http://jsfiddle.net/RQVsa/11/

Since the script hasn't yet been executed the .insertBefore() is making the ajax request to request the script, but since the hash is stripped it's never going to see the correct id.

@jaubourg, do you know why we're stripping off the hash? Is there a specific reason for it, or just an omission?

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 20, 2014

Member

Comment author: gibson042

A trip through the wayback machine reveals that we've been stripping hashes since 1.4.2's  fix for #4987.

I'd be in favor of preserving them, but if #3808 kills it for oldIE (and thus 1.9) then we may be unable to while maintaining API symmetry.

Member

mgol commented Oct 20, 2014

Comment author: gibson042

A trip through the wayback machine reveals that we've been stripping hashes since 1.4.2's  fix for #4987.

I'd be in favor of preserving them, but if #3808 kills it for oldIE (and thus 1.9) then we may be unable to while maintaining API symmetry.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 5, 2014

Member

Hmmm. Now that we are attempting to parse the URL a bit more sanely per gh-1875 it might be possible to preserve the hash now.

Member

dmethvin commented Dec 5, 2014

Hmmm. Now that we are attempting to parse the URL a bit more sanely per gh-1875 it might be possible to preserve the hash now.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 5, 2014

Member

By golly I think we can. After gh-1880 lands the hash is conveniently in ajaxLocation.hash and we could re-append it after adding any serialized params.

Member

dmethvin commented Dec 5, 2014

By golly I think we can. After gh-1880 lands the hash is conveniently in ajaxLocation.hash and we could re-append it after adding any serialized params.

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Dec 16, 2014

Member

This was unlocked by b091fdb, but we might want a distinct ticket for AJAX hash preservation.

Member

gibson042 commented Dec 16, 2014

This was unlocked by b091fdb, but we might want a distinct ticket for AJAX hash preservation.

@dmethvin dmethvin added this to the 3.0.0 milestone Dec 16, 2014

@dmethvin dmethvin added the Bug label Dec 16, 2014

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 16, 2014

Member

Agreed, since it's potentially a breaking change it's a good 3.0 candidate.

Member

dmethvin commented Dec 16, 2014

Agreed, since it's potentially a breaking change it's a good 3.0 candidate.

@dmethvin dmethvin changed the title from before() fails but native insertBefore() works ok to Preserve URL hash when requesting via $.ajax Dec 16, 2014

@dmethvin dmethvin self-assigned this Jan 5, 2015

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 10, 2015

Member

Looking at the WHATWG XHR spec they have intentionally taken the hash (fragment) off the URL in the responseURL property. @jaubourg do you know why that might be, or who we might ask? In the OP's reported case it was a script tag but I am wondering if there is more to the reasoning for excluding it for XHR.

Edit: Additional info here seems to indicate stripping the fragment early is okay: https://lists.w3.org/Archives/Public/public-webapps-github/2015Jul/1046.html

Member

dmethvin commented Nov 10, 2015

Looking at the WHATWG XHR spec they have intentionally taken the hash (fragment) off the URL in the responseURL property. @jaubourg do you know why that might be, or who we might ask? In the OP's reported case it was a script tag but I am wondering if there is more to the reasoning for excluding it for XHR.

Edit: Additional info here seems to indicate stripping the fragment early is okay: https://lists.w3.org/Archives/Public/public-webapps-github/2015Jul/1046.html

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Nov 11, 2015

Member

Fragments are never sent to servers, and are apparently not even exposed on fetches. But they can appear in the DOM and be visible to executing scripts. It's an extremely rare case to be sure, but one that script transports are technically wrong on as long as we strip them.

Member

gibson042 commented Nov 11, 2015

Fragments are never sent to servers, and are apparently not even exposed on fetches. But they can appear in the DOM and be visible to executing scripts. It's an extremely rare case to be sure, but one that script transports are technically wrong on as long as we strip them.

dmethvin added a commit to dmethvin/jquery that referenced this issue Nov 16, 2015

@modeckimellett

This comment has been minimized.

Show comment
Hide comment
@modeckimellett

modeckimellett Nov 19, 2015

Interesting. I asked a Stack Overflow question about this a while ago and never got a response. I'll add a link to this issue and the pull request to the question in comments. Thanks for working on this everyone!

For reference, my question is at http://stackoverflow.com/questions/32855458/jquery-ajax-content-from-an-end-point-which-responds-differently-depending-on-th

modeckimellett commented Nov 19, 2015

Interesting. I asked a Stack Overflow question about this a while ago and never got a response. I'll add a link to this issue and the pull request to the question in comments. Thanks for working on this everyone!

For reference, my question is at http://stackoverflow.com/questions/32855458/jquery-ajax-content-from-an-end-point-which-responds-differently-depending-on-th

@dmethvin dmethvin closed this in e077ffb Dec 1, 2015

@dmethvin dmethvin added the Ajax label Mar 9, 2016

scottgonzalez added a commit to jquery/jquery-ui that referenced this issue Jul 6, 2016

Tabs: Remove test for Ajax URLs containing hashes
This hasn't been a problem for a long time and jQuery no longer removes
the hash in 3.0.0, so the test started to fail even though the actual
code is working just fine.

Ref #3627
Ref jquery/jquery#1732

scottgonzalez added a commit to scottgonzalez/jquery-ui that referenced this issue Aug 30, 2016

Tabs: Strip hash from remote content URLs
As of jQuery 3.0.0, hashes are no longer stripped for Ajax requests. This
causes issues in IE <11, so we need to strip this before making the request.

Ref jquery/jquery#1732

scottgonzalez added a commit to jquery/jquery-ui that referenced this issue Aug 31, 2016

Tabs: Strip hash from remote content URLs
As of jQuery 3.0.0, hashes are no longer stripped for Ajax requests. This
causes issues in IE <11, so we need to strip this before making the request.

Ref jquery/jquery#1732
Closes gh-1736

@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.