This repository was archived by the owner on Apr 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 27.4k
refactor($sniffer): remove $sniffer.vendorPrefix #15287
Merged
Merged
+26
−77
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e577ba0
to
ac6d46e
Compare
LGTM - assuming that Travis is happy |
ac6d46e
to
2195091
Compare
2195091
to
3712c3f
Compare
The tests were failing in old WebKit, I fixed the PR. PTAL. |
LGTM |
Previously, Angular tried to detect the CSS prefix the browser supports and then use the saved one. This strategy is not ideal as currently some browsers are supporting more than one vendor prefix. The best example is Microsoft Edge that added -webkit- prefixes to be more Web-compatible; Firefox is doing a similar thing on mobile. Some of the -webkit--prefixed things are now even getting into specs to sanction that they're now required for Web compatibility. In some cases Edge even supports only the -webkit--prefixed property; one example is -webkit-appearance. $sniffer.vendorPrefix is no longer used in Angular core outside of $sniffer itself; taking that and the above problems into account, it's better to just remove it. The only remaining use case was an internal use in detection of support for transitions/animations but we can directly check the webkit prefix there manually; no other prefix matters for them anyway. $sniffer is undocumented API so this removal is not a breaking change. However, if you've previously been using it in your code, just paste the following to get the same function: var vendorPrefix = (function() { var prefix, prop, match; var vendorRegex = /^(Moz|webkit|ms)(?=[A-Z])/; for (prop in document.createElement('div').style) { if ((match = vendorRegex.exec(prop))) { prefix = match[0]; break; } } return prefix; })(); The vendorPrefix variable will contain what $sniffer.vendorPrefix used to. Note that we advise to not check for vendor prefixes this way; if you have to do it, it's better to check it separately for each CSS property used for the reasons described at the beginning. If you use jQuery, you don't have to do anything; it automatically adds vendor prefixes to CSS prefixes for you in the .css() method. Fixes angular#13690 Closes angular#15287
3712c3f
to
35482ba
Compare
ellimist
pushed a commit
to ellimist/angular.js
that referenced
this pull request
Mar 15, 2017
Previously, Angular tried to detect the CSS prefix the browser supports and then use the saved one. This strategy is not ideal as currently some browsers are supporting more than one vendor prefix. The best example is Microsoft Edge that added -webkit- prefixes to be more Web-compatible; Firefox is doing a similar thing on mobile. Some of the -webkit--prefixed things are now even getting into specs to sanction that they're now required for Web compatibility. In some cases Edge even supports only the -webkit--prefixed property; one example is -webkit-appearance. $sniffer.vendorPrefix is no longer used in Angular core outside of $sniffer itself; taking that and the above problems into account, it's better to just remove it. The only remaining use case was an internal use in detection of support for transitions/animations but we can directly check the webkit prefix there manually; no other prefix matters for them anyway. $sniffer is undocumented API so this removal is not a breaking change. However, if you've previously been using it in your code, just paste the following to get the same function: var vendorPrefix = (function() { var prefix, prop, match; var vendorRegex = /^(Moz|webkit|ms)(?=[A-Z])/; for (prop in document.createElement('div').style) { if ((match = vendorRegex.exec(prop))) { prefix = match[0]; break; } } return prefix; })(); The vendorPrefix variable will contain what $sniffer.vendorPrefix used to. Note that we advise to not check for vendor prefixes this way; if you have to do it, it's better to check it separately for each CSS property used for the reasons described at the beginning. If you use jQuery, you don't have to do anything; it automatically adds vendor prefixes to CSS prefixes for you in the .css() method. Fixes angular#13690 Closes angular#15287
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
removal
What is the current behavior? (You can also link to an open issue here)
$sniffer.vendorPrefix
is a vendor prefix of the current browser in the form used for properties inelement.style
, e.g.Moz
orWebkit
. It's an incorrect prefix in case of IE/Edge - it should bems
, notMs
there as this prefix is only available in the lowercase form.What is the new behavior (if this is a feature change)?
The API doesn't exist.
Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Other information:
Previously, Angular tried to detect the CSS prefix the browser supports and
then use the saved one. This strategy is not ideal as currently some browsers
are supporting more than one vendor prefix. The best example is Microsoft Edge
that added -webkit- prefixes to be more Web-compatible; Firefox is doing
a similar thing on mobile. Some of the -webkit--prefixed things are now even
getting into specs to sanction that they're now required for Web compatibility.
In some cases Edge even supports only the -webkit--prefixed property; one
example is -webkit-appearance.
$sniffer.vendorPrefix is no longer used in Angular core outside of $sniffer
itself; taking that and the above problems into account, it's better to just
remove it. The only remaining use case was an internal use in detection of
support for transitions/animations but we can directly check the webkit prefix
there manually; no other prefix matters for them anyway.
$sniffer is undocumented API so this removal is not a breaking change. However,
if you've previously been using it in your code, just paste the following
to get the same function:
The vendorPrefix variable will contain what $sniffer.vendorPrefix used to.
Note that we advise to not check for vendor prefixes this way; if you have to
do it, it's better to check it separately for each CSS property used for the
reasons described at the beginning. If you use jQuery, you don't have to do
anything; it automatically adds vendor prefixes to CSS prefixes for you in
the .css() method.
Fixes #13690