bug in ie when using datepicker with popup button and change focus

[ghost]

Using a datepicker with popup generates an error in Internet explorer 11.

It can be found in the demo site: https://angular-ui.github.io/bootstrap/#/datepicker and use the popup demo.
When using the datepicker (under the title Popup) clicking on the calendar button the calendar opens. If you click outside the datepicker area (to close the datepicker) you get the error : Object doesn't support property or method 'contains'. This is only internet explorer. I'm using ie 11.

[wesleycho]

For the curious, the offending line is https://github.com/angular-ui/bootstrap/blob/master/src/datepicker/datepicker.js#L716 - this is supposedly an issue with IE not having a contains method on nodes that are not attached to the DOM.

[smilix]

popupEl[0] is the html comment <!-- ngIf: isOpen --> and that kind of node doen't support contains here (in the IE browser). Use popupEl[0].nextSibling to get the expected <ul ...> element.

I fixed it by replacing https://github.com/angular-ui/bootstrap/blob/master/src/datepicker/datepicker.js#L716 with

var popupElWithContains = popupEl[0];
while (popupElWithContains !== null && !angular.isFunction(popupElWithContains.contains)) {
  popupElWithContains = popupElWithContains.nextSibling;
}
if (scope.isOpen && !(element[0].contains(event.target) || popupElWithContains.contains(event.target))) {

[yjukaku]

@smilix Yep, I see the same issue in IE10 and IE11.

I'd like to know the reason why #4316 even added the .contains() check, because it doesn't seem very clear.

Without really understanding why we are doing popupEl[0].contains(), I would suggest a simpler solution

var documentClickBind = function(event) {
        if (scope.isOpen && !( element[0].contains(event.target) || 
              ($popup[0].contains && $popup[0].contains(event.target)) ) { //only check the popup if it has a 'contains' function property
          scope.$apply(function() {
            scope.isOpen = false;
          });
        }
      };

[wesleycho]

That sounds reasonable to me. If you open a PR, we can get that merged quickly.

[Craig1f]

I fixed it like this:

Dev version:
change
|| popupEl[0].contains(event.target)
to
|| (popupEl[0].contains&&popupEl[0].contains(event.target))

Min version
change
||y[0].contains(a.target)
to
||y[0].contains&&(y[0].contains(a.target))

All we're doing is adding a null-safe check, so this shouldn't break any code that previously relied on the contains function. This also works on IE because it doesn't throw an exception if contains is not defined.

[chrisarcano]

Hello. May I know what version of angular bootstrap is this fix included?

[RobJacobs]

@chrisarcano If you look above where the issue was closed by commit 868c0e2, clicking on that link will take you to the commit detail page which lists the release the commit was part of: 1.1.0.

[chrisarcano]

I installed v1.3.2 using bower ( bower install angular-bootstrap ). I supposed the fix is included in that version, but upon checking and testing with an app, the fix is not present. Please advise.