refactor(jqLite): make HTML-parsing constructor more robust

[caitp]

Previously, the jqLite constructor was limited and would be unable to circumvent many of the HTML5
spec's "allowed content" policies for various nodes. This led to complicated and gross hacks around
this in the HTML compiler.

This change refactors these hacks by simplifying them, and placing them in jqLite rather than in
$compile, in order to better support these things, and simplify code.

While the new jqLite constructor is still not even close to as robust as jQuery, it should be more
than suitable enough for the needs of the framework, while adding minimal code.

Closes #6941
Closes #6926

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6958)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[caitp]

This looks like a bigger change than it really is. The wrapMap (essentially stolen from jQuery, credit where credit is due) greatly simplifies the hacks that previously lived in compile.js, and makes it much easier to extend in the future if needed. This is a big deal, IMO.

[IgorMinar]

this looks pretty good to me. I think we can go ahead and merge it in.

we should also run the tests on IE8 to see if it fails. if it doesn't we should consider cherry-picking it. if tests fail, we shouldn't bother with cherry-picking.

[aeharding]

Cool, thanks for doing this! I checked it out, and it almost all looks good. Of course, there is still the problem where transcluding and replacing a directive where the template is:

<select ng-transclude></select>

and the html using myDirective is:

<my-directive>
  <option>Option 1</option>
</my-directive>

So, it doesn't quite close #6926

[caitp]

Hmm, you're right... This is because for the markup to be valid, the element must be a <select>, and the template also contains <select>. So you end up asking for the select directive controller twice.

@IgorMinar is there a good way around this? I think that we have problems if we replace the controller with a new one, and we also have problems if we don't. So maybe there is nothing that can be done about that.

What if instantiation of the controller was delayed until all directives for the node were collected (IE, if the directive is a replace: true directive, wait before instantiating it). This way, it wouldn't try to instantiate two instances of the controller, and it wouldn't throw. Sounds handy.

I think, if this recent change works for IE8, then we can probably worry about that refactor later, if it is to happen, because I kind of just want to remove those crazy hacks from the compiler, and make them a bit more sane.

[caitp]

Good news, I've made this work in IE8... It might be a bit less performant than it otherwise would be, but it works, so we can backport it.

[IgorMinar]

Is the transclude+replace problem due to two instances of the select
directive on a single element? Or something else?
On Apr 2, 2014 1:27 PM, "Caitlin Potter" notifications@github.com wrote:

Good news, I've made this work in IE8... It might be a bit less performant
than it otherwise would be, but it works, so we can backport it.

[image: screen shot 2014-04-02 at 4 26 09 pm]https://cloud.githubusercontent.com/assets/2294695/2596269/1916d56a-baa5-11e3-8f7e-605014a90f88.png

Reply to this email directly or view it on GitHubhttps://github.com//pull/6958#issuecomment-39379139
.

[caitp]

You compile something like this:

<select some-directive-which-transcludes-so-that-options-stay-around>
  <option value="1">A</option>
  <option value="2">B</option>
</select>

That some-directive-which-transcludes-so-that-options-stay-around directive is a replace directive, whose root template element is a <select> tag. So the end result is, the <select> element gets compiled twice, and both times the select tag is treated as a directive and linked, and this causes the compiler to throw.

You don't really want to share the first controller with the new element, because it will possibly inject $element, and get the wrong one. You also don't want to allow keeping both of them, because this is potentially a leak, and is at the very least not inexpensive.

The ideal thing to do, I think, is, for replacing directives, wait until the template directives have been collected, before instantiating the controller. This way you can prevent linking the same thing twice.

But I think that is a fix that could be handled seperately.

[caitp]

@IgorMinar do you have time to give this another quick look today? (Ignoring the second commit, which is only needed for 1.2.x).

If it looks good, I'd like to get it into the release tomorrow.

[IgorMinar]

the first two commits look good.

we should ignore the issue with transclude+replace for select elements as long as it's not a regression.

[caitp]

Yeah, not a regression. I'll give this a merge then