fix(dropdown): support closest for HTMLDocument (#3783)

[Istanful]

Support closest for HTMLDocument

When autoclosing a dropdown by clicking the scrollbar, an error would be
thrown in Edge 44.18362.449.0. This was due to the HTMLDocument not
supporting Element.prototype.closest.

By emulating Element.prototype.closest for this specific node, this
commit circumvents the issue.

[benouat]

Nice to have added UT for that! This is great.
Nonetheless have a look at my comment below, and tell me what you think ?

[benouat]

I think we should do thing a little bit differently here because we can't just access document like that.

Any angular app could rendered server-side, where basically document won't be defined. So in order to properly use it, we usually have to get it by injection so that DI can provide a special one in environment where there is no document. (Have a look at how we do it)

But here, as we are in an utility function, it would mean we need to pass it as an argument, to have access to DI from the caller standpoint.

I found this a bit heavy, for such a small use case. (It apparently only fails in EdgeHTML)

What about we simply

try {
  return element.closest(selector)
} catch(err) { 
  return null;
}

WDYT ?

[Istanful]

Nice catch. It slipped my mind. 😛

I'm a bit hesitant to use try-catch since any error would be caught. How about using a more exact check?

if (typeof element.closest === 'undefined') {
  return null;
}

Beware though; in modern browsers document.documentElement.closest('html') would return document. By returning null always, we get a differing behaviour in Edge. How would you go about that problem?

[Istanful]

Ping @benouat. Would you be ok with that behaviour and check? :)

[benouat]

Yes, go ahead with that!

Don't forget to comment out things (this is needed for old edgeHTML generation) because I am pretty sure in 3 months from now we'll have forgotten why we do have this test!

[Istanful]

@benouat Cool. I've changed the commit and code according to discussion. :)

[benouat]

Could you please also rename the commit message when you'll push again with something starting with fix:
It's not build: related.

[Istanful]

@benouat Sure! I'll rename the commit. I wasn't sure what category it would fit in but figured :build from the description. :)

build: ... → any change for the utility scripts, configurations, dependencies, etc.

[benouat]

LGTM!