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

refactor($sniffer): remove $sniffer.vendorPrefix #15287

Merged
merged 1 commit into from
Oct 19, 2016

Conversation

mgol
Copy link
Member

@mgol mgol commented Oct 17, 2016

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 in element.style, e.g. Moz or Webkit. It's an incorrect prefix in case of IE/Edge - it should be ms, not Ms 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:

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 #13690

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Oct 17, 2016

LGTM - assuming that Travis is happy

@mgol mgol changed the title removal($sniffer): remove $sniffer.vendorPrefix refactor($sniffer): remove $sniffer.vendorPrefix Oct 17, 2016
@mgol mgol force-pushed the vendorPrefix-removal branch from ac6d46e to 2195091 Compare October 17, 2016 21:21
@mgol mgol force-pushed the vendorPrefix-removal branch from 2195091 to 3712c3f Compare October 17, 2016 21:46
@mgol
Copy link
Member Author

mgol commented Oct 17, 2016

The tests were failing in old WebKit, I fixed the PR. PTAL.

@petebacondarwin
Copy link
Contributor

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
@mgol mgol force-pushed the vendorPrefix-removal branch from 3712c3f to 35482ba Compare October 19, 2016 21:11
@mgol mgol merged commit 35482ba into angular:master Oct 19, 2016
@mgol mgol deleted the vendorPrefix-removal branch October 19, 2016 21:35
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove $sniffer.vendorPrefix (was: Rethink the CSS prefixing strategy)
3 participants