fix($parse): reduce false-positives in isElement tests #5675
Conversation
There are always going to be false positives here, unfortunately. But testing different properties will hopefully reduce the number of false positives in a meaningful way, without harming performance too much. Closes angular#4805
|
Those aren't really related to this issue, I think they can be fixed seperately, unless it's super important to people to keep those in sync |
|
@caitp I think it should be in sync; the one you fixed is actually an |
|
yes, I'm aware --- I'm not opposed to making the change, but it's not really atomic to this particular bug, and it's usually nice to have good atomicity in changesets so that it's very clear what is changed and why. I can open another PR with those, or add another commit to this one, depending on what people think is the better approach. @IgorMinar sorry to ping you, do you have a preference for this? |
|
That makes a point; perhaps just add another commit? |
ghost
assigned
|
I'll squash or remove the commit if it's asked for |
Complimentary change to match changed $parse behaviour.
| @@ -569,7 +569,7 @@ var trim = (function() { | |||
| function isElement(node) { | |||
| return !!(node && | |||
| (node.nodeName // we are a direct element | |||
| || (node.on && node.find))); // we have an on and find method part of jQuery API | |||
| || (node.prop && node.attr && node.find))); // we have an on and find method part of jQuery API | |||
There was a problem hiding this comment.
this isn't really in sync with the stuff in $parse, and wasn't to begin with (not looking at the children property). Adding it actually breaks a lot of tests (on travis, seems to pass everything locally). Hmm.
|
lgtm |
There are always going to be false positives here, unfortunately. But testing
different properties will hopefully reduce the number of false positives in a
meaningful way, without harming performance too much.
Closes #4805