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

refactor($sce): Use $sniffer instead of $document for feature detection. #5045

Closed
wants to merge 1 commit into from

Conversation

tbosch
Copy link
Contributor

@tbosch tbosch commented Nov 20, 2013

Also adds $sniffer.msieDocumentMode property.

Fixes #4931.

@petebacondarwin
Copy link
Member

LGTM

'mode. You can fix this by adding the text <!doctype html> to the top of your HTML ' +
'document. See http://docs.angularjs.org/api/ng.$sce for more information.');
}
if (enabled && $sniffer.msie && isDefined($sniffer.msieDocumentMode) && $sniffer.msieDocumentMode < 8) {
Copy link
Member

Choose a reason for hiding this comment

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

This line does break the 100 character limit. It might be worth breaking it up to make it more manageable.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we even need isDefined($sniffer.msieDocumentMode)?

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 is not - really - since undefined < 8 is true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, just copied over the old code.

Copy link
Member

Choose a reason for hiding this comment

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

Actually the fact that undefined<8 is true means that you do need the isDefined check to get the right result, i.e. that it is msie and documentMode exists and is less than 8.

But if it is msie then documentMode will always exist so in the end the line is OK.

@petebacondarwin
Copy link
Member

While we are at it, on line https://github.com/angular/angular.js/blob/master/src/ng/sce.js#L202, $log and $document are never used.

@IgorMinar
Copy link
Contributor

+1 to what pete said, otherwise lgtm

@ghost ghost assigned petebacondarwin Nov 20, 2013
@tbosch
Copy link
Contributor Author

tbosch commented Nov 20, 2013

Thanks!

Also adds `$sniffer.msieDocumentMode` property.

Fixes angular#4931.
@tbosch tbosch closed this in ec3c4f9 Nov 20, 2013
@tbosch tbosch deleted the sniffer-sce-documentmode branch November 20, 2013 23:47
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test fails in IE: TypeError: Unable to get property 'documentMode' of undefined or null reference
3 participants