-
Notifications
You must be signed in to change notification settings - Fork 27.4k
refactor($sce): Use $sniffer instead of $document for feature detection. #5045
Conversation
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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
While we are at it, on line https://github.com/angular/angular.js/blob/master/src/ng/sce.js#L202, |
+1 to what pete said, otherwise lgtm |
Thanks! |
Also adds `$sniffer.msieDocumentMode` property. Fixes angular#4931.
Also adds `$sniffer.msieDocumentMode` property. Closes angular#4931 Closes angular#5045
Also adds `$sniffer.msieDocumentMode` property. Closes angular#4931 Closes angular#5045
Also adds
$sniffer.msieDocumentMode
property.Fixes #4931.