Skip to content

$.parseHTML() creates anchor elements with empty href property in 1.12/2.2 in Chrome #2941

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

Closed
vmagik opened this issue Feb 23, 2016 · 20 comments
Assignees
Milestone

Comments

@vmagik
Copy link

vmagik commented Feb 23, 2016

$.parseHTML() seems to be broken in jQuery v1.12.0 and later for Chrome
This code shows empty string in Chrome but it is "test.html" in other browsers.
In jQuery 1.11.3 Chrome was OK too.

<script type="text/javascript" src="jquery-1.12.1.js"></script>
<script type="text/javascript">
    function test()
    {
        var html = '<a href="test.html"></a>';
        var ref = $.parseHTML(html);

        alert(ref[0].href);
    }
</script>

EDIT (@mgol): I modified the alert to alert the href property of the anchor element; originally the test case was alerting the element (alert(ref)) which meant it relied on its stringification.

@dmethvin
Copy link
Member

The result of jQuery.parseHTML should be some DOM, in this case an <a> element. It seems to be correct.

http://jsbin.com/balozatego/1/edit?js,console

Do you have code that depends on stringifying DOM elements as you do in the example?

@timmywil
Copy link
Member

Closing as worksforme. Please provide a test case to reopen.

@mgol
Copy link
Member

mgol commented Feb 24, 2016

@vmagik Thanks for the update, we had an impression this report is about stringifying DOM elements but it seems the href property is, indeed, set to an empty string in Chrome. We need to investigate further.

@mgol mgol reopened this Feb 24, 2016
@mgol
Copy link
Member

mgol commented Feb 24, 2016

OK, I nailed down the issue to commit 58c2460. Here's a non-jQuery test case:

var doc = document.implementation.createHTMLDocument();
var div = doc.createElement('div')
div.innerHTML = '<a href="foo.html"></a>';
document.body.innerHTML = 'URL: ' + div.childNodes[0].href;

Also posted at https://jsfiddle.net/ebacz2u5/2/. This test case fails in Chrome 48 and succeeds in Firefox 44, Safari 9.0, Edge 13, IE 9-11 (all that support document.implementation.createHTMLDocument), Opera 12.16 and even Android 2.3.

I see it reported upstream at https://bugs.chromium.org/p/chromium/issues/detail?id=568886

@mgol mgol changed the title $.parseHTML() is broken in 1.12.0 and 1.12.1 for Chrome $.parseHTML() creates anchor elements with empty href property in 1.12/2.2 in Chrome Feb 24, 2016
@dmethvin
Copy link
Member

@vmagik How are you using this code in your real application? Are you calling $.parseHTML yourself or is something else calling it for you? We're trying to determine the impact of this.

@vmagik
Copy link
Author

vmagik commented Feb 24, 2016

I call $.parseHTML() myself.
My code gets a string like "<a ...</a>" and tries to get href from it. I didn't invent the code in the first message (with stringfication). Found an example somewhere.

@dmethvin
Copy link
Member

What are you doing with the result though? Again we're trying to determine the seriousness of the bug based on the way things are being used in real code.

@vmagik
Copy link
Author

vmagik commented Feb 24, 2016

I open a window to show resulting href. Also can use it for ajax call

@dmethvin
Copy link
Member

You mean to show it to the user? I'm trying to get a sense of the work flow that is occurring here. Some type of HTML is parsed, where does it come from? Why is the href shown in a window?

@vmagik
Copy link
Author

vmagik commented Feb 24, 2016

I show it to the user.
On click on some link on the page I first do ajax request which can result either in one link or several links.
If it's one link I just show it in a new window or a dialog. If several links then I need to show user a choice.

@timmywil
Copy link
Member

I guess if there's no workaround, we'll need to back out document.implementation in master too.

@timmywil timmywil added this to the 1.12.2/2.2.2 milestone Feb 24, 2016
@timmywil
Copy link
Member

Start with removal in 1.12.2/2.2.2. We're going to wait on removing in 3.0.

@timmywil timmywil self-assigned this Feb 29, 2016
@gibson042
Copy link
Member

This seems to be fixable by something as simple as

var base = createdDoc.head.appendChild( createdDoc.createElement( "base" ) );
base.href = document.location.href;

Dirty, but not the first time we've had to spackle over browser behavior.

@timmywil
Copy link
Member

@gibson042 Nice!

@dmethvin
Copy link
Member

Since the caller can pass in a context of any document, does it work to set the base to an empty string? That way the href property/attribute would be equal.

I still would suggest we back this out in 1.x/2.x because we aren't using it consistently and it's not documented.

@mgol
Copy link
Member

mgol commented Feb 29, 2016

I like where @gibson042's going and I'd like to experiment with it on master but at this point all such changes seem risky to me for 1.12/2.2 which we're trying to stop being developed soon so I agree with @dmethvin we should back it out for 1.12.2/2.2.2.

@BobVul
Copy link

BobVul commented Mar 2, 2016

Just saw this from the Chromium bugtracker. Interesting that it's affecting a larger library.

@vmagik - are you sure you're getting the resolved URL in Firefox? Your JSBin link shows just "test.html" in Firefox 44 and 46, the full "http://null.jsbin.com/test.html" in IE11 and "" in Chrome 48 for me.

@gibson042's fix looks like the correct way to go about it. Based on the brief look I had at the specs, the default base URL is about:blank, so if your intention is to make the behaviour match the document fallback then you'd need to set the base URL anyway. Even if the Chromium bug is fixed correctly, you'd still need to do this to match document behaviour.

mgol added a commit that referenced this issue Mar 2, 2016
The document.implementation.createHTMLDocument("") method creates inert
documents which is good but using it has introduced issues around anchor
elements href property not resolving according to the current document.
Because of that, this patch is getting backed out on 1.x/2.x branches.

Refs cfe468f
Refs gh-1505
Fixes gh-2941
mgol added a commit that referenced this issue Mar 2, 2016
The document.implementation.createHTMLDocument("") method creates inert
documents which is good but using it has introduced issues around anchor
elements href property not resolving according to the current document.
Because of that, this patch is getting backed out on 1.x/2.x branches.

(cherry-picked from c5c3073)

Refs cfe468f
Refs gh-1505
Fixes gh-2941
@mgol
Copy link
Member

mgol commented Mar 2, 2016

The backout has landed in c5c3073 on 2.2-stable & 6403cf6 for 1.12-stable. This issue is now resolved.

I created a separate one for making it work in 3.0: #2965.

@mgol mgol closed this as completed Mar 2, 2016
@vmagik
Copy link
Author

vmagik commented Mar 6, 2016

@Elusive138 Yes, Firefox 44 shows just "test.html" with jQuery 1.12.x. But with jQuery 1.11.3 it shows resolved URL.
http://jsbin.com/zuxevusoxi/edit?js,console
But I think this is not a big problem if a problem at all.

@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.
Development

No branches or pull requests

6 participants