fix(jqLite): clone wrapNode in jqlite/wrap #4194
Conversation
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
I just signed, my real name is Chris Rhoden |
|
I wrote tests and ran them, and they passed locally. It looks like the travis failure here is the result of a problem with an external service. Is there a way for me to re-run this? |
|
Travis is unreliable right now, so don't worry too much about it. |
|
+1 |
|
@chrisrhoden Can you rebase it against master? I'll get to it within a few days. |
|
(changed the milestone to the next 1.3.0 beta) |
Change jqLite's implementation of wrap() to clone the wrapNode before wrapping the target element in it. Match jQuery's wrap() behavior and prevent accidentally attaching target element to the DOM as a side effect. Closes angular#3860
|
@mzgol do we need this for jQuery 2.x support? Please answer quickly otherwise this will be pushed back to the next milestone |
|
@caitp No, this behavior hasn't changed between jQuery 1.x and 2.x, so to the extent that jqLite is currently broken as a replacement for 1.x, it will be no more broken as a replacement for 2.x (at least as it applies to this patch). |
| it('should clone elements to be wrapped around target', function () { | ||
| var root = jqLite('<div class="sigil"></div>'); | ||
| var span = jqLite('<span>A</span>'); | ||
| jqLite(document.body).append(root); |
There was a problem hiding this comment.
Why is this needed? Couldn't you use any other wrapper for root that is not actually in the DOM?
|
thanks for the PR! |
Change jqLite's implementation of wrap() to clone the wrapNode before
wrapping the target element in it.
Match jQuery's wrap() behavior and prevent accidentally attaching
target element to the DOM as a side effect.
Closes #3860