Bug Tracker

Opened 6 years ago

Closed 6 years ago

#14846 closed bug (invalid)

$.buildFragment does not correctly include whitespace in IE8

Reported by: justinbmeyer@… Owned by: justinbmeyer@…
Priority: undecided Milestone: None
Component: unfiled Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:

Description

Here's a fiddle demonstrating the issue: http://jsfiddle.net/p6Way/1/. As JSFiddle doesn't work in IE8, you can open http://fiddle.jshell.net/p6Way/1/show/ to see the problem.

Essentially, text like: "<-\n>" is not properly converted to a text node with the equivalent nodeValue in IE8.

A possible fix:

After setting the innerHTML here:

tmp.innerHTML = wrap[1] + elem.replace( rxhtmlTag, "<$1></$2>" ) + wrap[2];

Check if only one child node was created. If only a single childNode was created, there were no valid html tags. Therefore, simply create a text node with it's nodeValue as the elem.

An alternative would be to improve the rhtml regular expression to match HTML better.

I'm happy to submit a fix for this if this is something you are interested in fixing. Implementing the mustache spec depends on this character combination working.

Change History (5)

comment:1 Changed 6 years ago by dmethvin

Is Mustache calling $.buildFragment directly? It's not a public API and might change. We did a lot of refactoring internally in the 1.9 timeframe for example.

To the point, though, that regex has been troublesome but implements a feature that we can't take out. At least your proposed fix doesn't require changing the regex.

comment:2 Changed 6 years ago by anonymous

What feature does it implement (other than toggling between simply creating a text node or using the .innerHTML trick)?

I'm currently patching $.buildFragment like:

var text = "<-\n>";
var frag = $.buildFragment([text], document);
if(text !== frag.childNodes[0].nodeValue) {
	
	var oldBuildFragment  = $.buildFragment;
	$.buildFragment = function(content, context){
		var res = oldBuildFragment(content, context);
		if(res.childNodes.length === 1 && res.childNodes[0].nodeType === 3) {
			res.childNodes[0].nodeValue = content[0];
		} 
		return res;
	}
}

This way, only browsers that get things wrong are affected.

In regards to non-public APIs ... Afaik, there's not a great and as fast alternative that jQuery provides. We have a computability layer so if the API changes, we will update things accordingly.

We actually "feature detect" how $.fn.domManip is called and hook into it differently depending on which version of jQuery we are running against.

comment:3 Changed 6 years ago by gibson042

The proposed approach will fail when the HTML-resembling text is buried in a string of actual HTML (e.g., jQuery.html("<div><-\n></div>")). This looks to me like another use case for #14228.

Also, out of curiosity: why not HTML-escape or CDATA-wrap the problematic text characters? Is https://github.com/mustache/spec/blob/v1.1.2/specs/~lambdas.yml#L100 in particular the source of your problem?

comment:4 Changed 6 years ago by dmethvin

Owner: set to justinbmeyer@…
Status: newpending

Let's keep the discussion going on this, if needed.

comment:5 Changed 6 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Note: See TracTickets for help on using tickets.