fix(jqLite): do not break if Node is not available

[gkalpak]

Fixes #13442

[gkalpak]

Does anyone have any idea how/if we can test that it works if Node is not availalbe on the global context ?

[petebacondarwin]

Did we identify for which browser this breaks? If so then we should be able to reproduce the problem in a unit test easily enough by including that browser in our testing...

[gkalpak]

If I understood the usecase in #13442 correctly, this isn't fixing a browser issue, but some specific setup issue (with requirejs, maybe server-side stuff), where Node was not available in the global context. I guess we can't replicate that in tests.

The original fix that was solved by 941c1c3 (which introduced the dependecy on Node) was affecting IE9-11. I tried on IE11 and there are indeed unit tests that fail without that fix, but pass with this commit.

We are already testing against IE9-11, so we would be good to go in that respect.

[petebacondarwin]

I would rather not do the check on every call to jqLiteContains... How about the following?

var jqLiteContains = $window.Node && $window.Node.prototype.contains || function(arg) {
...

[gkalpak]

I thought about that too, but I wanted to use the "native" contains as much as possible. E.g. when Node is undefined at the time of defining jqLiteContains (for whatever reason, which isn't necessarily that .contains() is not supported by the environment), it will use our custom method and not the native one (even if it's available on target).

Besides, in the majority of cases (where Node.prototype.contains will be available), you will still be calling jqLiteContains.call(target, ...) which I figured wouldn't be much of an optimization over target.contains && target.contains(...). (I am just making assumptions here, I didn't run any benchmarks, since it isn't a critical path anyway).

But, yeah, I could have gone with your version, I just had to pick one.
I'm fine changing it.

[petebacondarwin]

I don't understand when window.Node could be undefined only temporarily.

[gkalpak]

I'm not sure either (maybe @jlmitch5 or @neha-nsharma can shed more light on this).
What I suspect is that (somehow) the window is set up at a later time (after angular.js has been executed). See #13442 (comment).

Maybe using a JS DOM implementation for testing (e.g. jsdom).

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

[gkalpak]

I signed it (?)

[petebacondarwin]

I am loathe to land this without a more concrete example of where this is failing.

[gkalpak]

@petebacondarwin, it was also reported in #13391. It seems indeed to be failing with jsdom (among other things ?).

[googlebot]

CLAs look good, thanks!

[petebacondarwin]

It looks to me that you can define a window object via jsdom and add it to the global context (https://github.com/tmpvar/jsdom#for-the-hardcore-jsdomjsdom and https://github.com/tmpvar/jsdom#creating-a-browser-like-window-object).

and that Node ought to be available on that window (https://github.com/tmpvar/jsdom/blob/2184995dc6531cb92d219497c8f88c9b7f088bc1/lib/jsdom/living/node.js).

If not then I would raise a bug in jsdom.

Can anyone provide an actual running example of this problem?

[gkalpak]

@petebacondarwin, according to #13442 (comment) (and subsequent discussion), it seems that changing Node.prototype.contains to window.Node.ptototype.contains solves the issue.

It sounds a reasonable change to me. WDYT ?

[petebacondarwin]

I need to see an actual real running example where this is breaking.

[petebacondarwin]

Closing in favour of a deeper investigation from @mgol of the other possible ways that using jsdom or similar could break AngularJS 1.