Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(*): cleanup msie handling; add support comments #15407

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ var
angularModule,
uid = 0;

// Support: IE 9-11 only
/**
* documentMode is an IE-only property
* http://msdn.microsoft.com/en-us/library/ie/cc196988(v=vs.85).aspx
Expand Down Expand Up @@ -1279,6 +1280,7 @@ function fromJson(json) {

var ALL_COLONS = /:/g;
function timezoneToOffset(timezone, fallback) {
// Support: IE 9-11 only, Edge 13-14+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between Edge 13-14+ and Edge 13+?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the former we've checked that it, indeed, is broken in both v13 & v14. In the latter we've just checked v13 and the v14 state is unknown.

// IE/Edge do not "understand" colon (`:`) in timezone
timezone = timezone.replace(ALL_COLONS, '');
var requestedTimezoneOffset = Date.parse('Jan 01, 1970 00:00:00 ' + timezone) / 60000;
Expand Down
3 changes: 2 additions & 1 deletion src/auto/injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,9 @@ function createInjector(modulesToLoad, strictDi) {
}

function isClass(func) {
// Support: IE 9-11 only
// IE 9-11 do not support classes and IE9 leaks with the code below.
if (msie <= 11 || typeof func !== 'function') {
if (msie || typeof func !== 'function') {
return false;
}
var result = func.$$ngIsClass;
Expand Down
1 change: 1 addition & 0 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
for (var i = 0; i < nodeList.length; i++) {
attrs = new Attributes();

// Support: IE 11 only
// Workaround for #11781 and #14924
if (msie === 11) {
mergeConsecutiveTextNodes(nodeList, i, notLiveList);
Expand Down
5 changes: 3 additions & 2 deletions src/ng/directive/attrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,11 @@ forEach(['src', 'srcset', 'href'], function(attrName) {

attr.$set(name, value);

// on IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
// Support: IE 9-11 only
// On IE, if "ng:src" directive declaration is used and "src" attribute doesn't exist
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
// to set the property as well to achieve the desired effect.
// we use attr[attrName] value since $set can sanitize the url.
// We use attr[attrName] value since $set can sanitize the url.
if (msie && propName) element.prop(propName, attr[name]);
});
}
Expand Down
1 change: 1 addition & 0 deletions src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function $RootScopeProvider() {

function cleanUpScope($scope) {

// Support: IE 9 only
if (msie === 9) {
// There is a memory leak in IE9 if all child scopes are not disconnected
// completely when a scope is destroyed. So this code will recurse up through
Expand Down
1 change: 1 addition & 0 deletions src/ng/sce.js
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@ function $SceProvider() {

this.$get = ['$parse', '$sceDelegate', function(
$parse, $sceDelegate) {
// Support: IE 9-11 only
// Prereq: Ensure that we're not running in IE<11 quirks mode. In that mode, IE < 11 allow
// the "expression(javascript expression)" syntax which is insecure.
if (enabled && msie < 8) {
Expand Down
3 changes: 2 additions & 1 deletion src/ng/sniffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,13 @@ function $SnifferProvider() {
// We are purposefully using `!(android < 4)` to cover the case when `android` is undefined
history: !!(hasHistoryPushState && !(android < 4) && !boxee),
hasEvent: function(event) {
// Support: IE 9-11 only
// IE9 implements 'input' event it's so fubared that we rather pretend that it doesn't have
// it. In particular the event is not fired when backspace or delete key are pressed or
// when cut operation is performed.
// IE10+ implements 'input' event but it erroneously fires under various situations,
// e.g. when placeholder changes, or a form is focused.
if (event === 'input' && msie <= 11) return false;
if (event === 'input' && msie) return false;

if (isUndefined(eventSupport[event])) {
var divElm = document.createElement('div');
Expand Down
1 change: 1 addition & 0 deletions src/ng/urlUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ var originUrl = urlResolve(window.location.href);
function urlResolve(url) {
var href = url;

// Support: IE 9-11 only
if (msie) {
// Normalize before parse. Refer Implementation Notes on why this is
// done in two steps on IE.
Expand Down
7 changes: 7 additions & 0 deletions src/ngScenario/dsl.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ angular.scenario.dsl('binding', function() {
*/
angular.scenario.dsl('input', function() {
var chain = {};

// Support: IE 9-11 only
// IE9 implements 'input' event it's so fubared that we rather pretend that it doesn't have
// it. In particular the event is not fired when backspace or delete key are pressed or
// when cut operation is performed.
// IE10+ implements 'input' event but it erroneously fires under various situations,
// e.g. when placeholder changes, or a form is focused.
var supportInputEvent = 'oninput' in window.document.createElement('div') && !msie;

chain.enter = function(value, event) {
Expand Down
5 changes: 5 additions & 0 deletions test/helpers/matchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,19 @@ beforeEach(function() {

toBeMarkedAsSelected: function() {
// Selected is special because the element property and attribute reflect each other's state.

// Support: IE 9 only
// IE9 will wrongly report hasAttribute('selected') === true when the property is
// undefined or null, and the dev tools show that no attribute is set

return {
compare: function(actual) {
var errors = [];
if (actual.selected === null || typeof actual.selected === 'undefined' || actual.selected === false) {
errors.push('Expected option property "selected" to be truthy');
}

// Support: IE 9 only
if (msie !== 9 && actual.hasAttribute('selected') === false) {
errors.push('Expected option to have attribute "selected"');
}
Expand All @@ -383,6 +387,7 @@ beforeEach(function() {
errors.push('Expected option property "selected" to be falsy');
}

// Support: IE 9 only
if (msie !== 9 && actual.hasAttribute('selected')) {
errors.push('Expected option not to have attribute "selected"');
}
Expand Down
16 changes: 13 additions & 3 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,10 @@ describe('$compile', function() {
}));

// NOTE: This test may be redundant.
// Support: Edge 14+
// An `<svg>` element inside a `<foreignObject>` element on MS Edge has no
// size, causing the included `<circle>` element to also have no size and thus fails an
// assertion (relying on the element having a non-zero size).
if (!isEdge) {
it('should handle custom svg containers that transclude to foreignObject' +
' that transclude to custom svg containers that transclude to custom elements', inject(function() {
Expand Down Expand Up @@ -1127,7 +1131,8 @@ describe('$compile', function() {
expect(element).toHaveClass('class_2');
}));

if (!msie || msie > 11) {
// Support: IE 9-11 only
if (!msie) {
// style interpolation not working on IE (including IE11).
it('should handle interpolated css style from replacing directive', inject(
function($compile, $rootScope) {
Expand Down Expand Up @@ -10698,6 +10703,8 @@ describe('$compile', function() {
expect(element.text()).toBe('102030');
expect(newWatcherCount).toBe(3);

// Support: IE 11 only
// See #11781 and #14924
if (msie === 11) {
expect(element.find('ng-transclude').contents().length).toBe(1);
}
Expand All @@ -10719,9 +10726,10 @@ describe('$compile', function() {
expect(element.attr('src')).toEqual('http://example.com/image2.png');
}));

// Support: IE 9 only
// IE9 rejects the video / audio tag with "Error: Not implemented" and the source tag with
// "Unable to get value of the property 'childNodes': object is null or undefined"
if (!msie || msie > 9) {
if (msie !== 9) {
they('should NOT require trusted values for $prop src', ['video', 'audio'],
function(tag) {
inject(function($rootScope, $compile, $sce) {
Expand Down Expand Up @@ -11154,7 +11162,9 @@ describe('$compile', function() {
}));
});

if (!msie || msie >= 11) {
// Support: IE 9-10 only
// IEs <11 don't support srcdoc
if (!msie || msie === 11) {
describe('iframe[srcdoc]', function() {
it('should NOT set iframe contents for untrusted values', inject(function($compile, $rootScope, $sce) {
element = $compile('<iframe srcdoc="{{html}}"></iframe>')($rootScope);
Expand Down
6 changes: 4 additions & 2 deletions test/ng/directive/booleanAttrsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ describe('ngSrc', function() {
}));


// Support: IE 9-11 only
if (msie) {
it('should update the element property as well as the attribute', inject(
function($compile, $rootScope, $sce) {
Expand Down Expand Up @@ -283,8 +284,9 @@ describe('ngHref', function() {
expect(element.attr('href')).toEqual(undefined);
}));

if (msie) {
// IE11/10/Edge fail when setting a href to a URL containing a % that isn't a valid escape sequence
// Support: IE 9-11 only, Edge 12-14+
if (msie || /\bEdge\/[\d\.]+\b/.test(window.navigator.userAgent)) {
// IE/Edge fail when setting a href to a URL containing a % that isn't a valid escape sequence
// See https://github.com/angular/angular.js/issues/13388
it('should throw error if ng-href contains a non-escaped percent symbol', inject(function($rootScope, $compile) {
element = $compile('<a ng-href="http://www.google.com/{{\'a%link\'}}">')($rootScope);
Expand Down
1 change: 1 addition & 0 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ describe('input', function() {


describe('IE placeholder input events', function() {
// Support: IE 9-11 only
//IE fires an input event whenever a placeholder visually changes, essentially treating it as a value
//Events:
// placeholder attribute change: *input*
Expand Down
16 changes: 10 additions & 6 deletions test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ describe('$location', function() {
/* global urlParsingNode: true */
var urlParsingNodePlaceholder;

beforeEach(inject(function($sniffer) {
if (msie) return;
beforeEach(function() {
// Support: non-Windows browsers
// These tests expect a Windows environment which we can only guarantee
// on IE & Edge.
if (msie || /\bEdge\/[\d\.]+\b/.test(window.navigator.userAgent)) return;

urlParsingNodePlaceholder = urlParsingNode;

Expand All @@ -57,13 +60,14 @@ describe('$location', function() {
search: '',
setAttribute: angular.noop
};
}));
});

afterEach(inject(function($sniffer) {
if (msie) return;
afterEach(function() {
// Support: non-Windows browsers
if (msie || /\bEdge\/[\d\.]+\b/.test(window.navigator.userAgent)) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have this as a reusable flag in tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it doesn't hurt when it's used just in tests... (I would definitely not want sth like that in source). But would you like to have it in tests globally or just in locationSpec? If globally then we should define similar variables for other browsers.

I'm a little afraid of making it too easy to use UA-sniffing, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't hurt. It is just about the mental overhead of "deciphering" /\bEdge\/[\d\.]+\b/.test(window.navigator.userAgent) vs isEdge.

I'm a little afraid of making it too easy to use UA-sniffing, though.

Fair enough 😃

//reset urlParsingNode
urlParsingNode = urlParsingNodePlaceholder;
}));
});


it('should not include the drive name in path() on WIN', function() {
Expand Down
3 changes: 2 additions & 1 deletion test/ng/parseSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2180,8 +2180,9 @@ describe('parser', function() {
expect(scope.$eval('getter()()')).toBe(33);
});

// Support: IE 9 only
// There is no "strict mode" in IE9
if (!msie || msie > 9) {
if (msie !== 9) {
it('should set no context to functions returned by other functions', function() {
scope.getter = function() { return function() { expect(this).toBeUndefined(); }; };
scope.$eval('getter()()');
Expand Down
9 changes: 5 additions & 4 deletions test/ng/rootScopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ describe('Scope', function() {


it('should expose the constructor', inject(function($rootScope) {
if (msie < 11) return;
// eslint-disable-next-line no-proto
expect($rootScope.__proto__).toBe($rootScope.constructor.prototype);
expect(Object.getPrototypeOf($rootScope)).toBe($rootScope.constructor.prototype);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the msie < 11 check gone?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was there only because of the Annex B (i.e. compatibility "don't use" stuff) __proto__ that wasn't present in IE <11. The standard ES5+ way to get the prototype is Object.getPrototypeOf(object) which works in IE9+.

}));


Expand Down Expand Up @@ -125,7 +123,9 @@ describe('Scope', function() {
function Listener() {
expect(this).toBeUndefined();
}
if (msie < 10) return;
// Support: IE 9 only
// IE 9 doesn't support strict mode so its `this` will always be defined.
if (msie === 9) return;
$rootScope.$watch(Getter, Listener);
$rootScope.$digest();
}));
Expand Down Expand Up @@ -1227,6 +1227,7 @@ describe('Scope', function() {
}));


// Support: IE 9 only
if (msie === 9) {
// See issue https://github.com/angular/angular.js/issues/10706
it('should completely disconnect all child scopes on IE9', inject(function($rootScope) {
Expand Down
3 changes: 2 additions & 1 deletion test/ng/snifferSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,12 @@ describe('$sniffer', function() {


it('should claim that IE9 doesn\'t have support for "oninput"', function() {
// Support: IE 9-11 only
// IE9 implementation is fubared, so it's better to pretend that it doesn't have the support
// IE10+ implementation is fubared when mixed with placeholders
mockDivElement = {oninput: noop};

expect($sniffer.hasEvent('input')).toBe(!(msie && msie <= 11));
expect($sniffer.hasEvent('input')).toBe(!msie);
});
});

Expand Down