fix(ripple): Feature-detect buggy Edge behavior for custom properties (…

…#1041)

Also changes the supportsCssVariables util method to remember its result so it doesn't need to execute its logic every time a ripple is initialized.

packages/mdc-ripple/README.md

@@ -31,6 +31,7 @@ path: /catalog/ripples/
     - [Using a sentinel element for a ripple](#using-a-sentinel-element-for-a-ripple)
     - [Keyboard interaction for custom UI components](#keyboard-interaction-for-custom-ui-components)
     - [Specifying known element dimensions](#specifying-known-element-dimensions)
+  - [Caveat: Edge](#caveat-edge)
   - [Caveat: Safari](#caveat-safari)
   - [Caveat: Theme Custom Variables](#caveat-theme-custom-variables)
   - [The util API](#the-util-api)
@@ -45,9 +46,7 @@ In order to function correctly, MDC Ripple requires a _browser_ implementation o
 
 Because we rely on scoped, dynamic CSS variables, static pre-processors such as [postcss-custom-properties](https://github.com/postcss/postcss-custom-properties) will not work as an adequate polyfill ([...yet?](https://github.com/postcss/postcss-custom-properties/issues/32)).
 
-[Most modern browsers](http://caniuse.com/#feat=css-variables) support CSS variables, so MDC ripple will work just fine. In other cases, MDC ripple will _still work_ if you include it in your codebase. It will simply check if CSS variables are supported upon initialization and if they aren't, gracefully exit. The only exception to this rule is Safari, which does support CSS variables
-but unfortunately ripples are disabled for (see [below](#caveat-safari) for an explanation).
-
+Edge and Safari, although they do [support CSS variables](http://caniuse.com/#feat=css-variables), do not support MDC Ripple. See the respective caveats for [Edge](#caveat-edge) and [Safari](#caveat-safari) for an explanation.
 
 ## Installation
 
@@ -263,7 +262,7 @@ ripple to. The adapter API is as follows:
 
 | Method Signature | Description |
 | --- | --- |
-| `browserSupportsCssVars() => boolean` | Whether or not the given browser supports CSS Variables. When implementing this, please take the [Safari considerations](#caveat-safari) into account. We provide a `supportsCssVariables` function within the `util.js` which we recommend using, as it handles this for you. |
+| `browserSupportsCssVars() => boolean` | Whether or not the given browser supports CSS Variables. When implementing this, please take the [Edge](#caveat-edge) and [Safari](#caveat-safari) considerations into account. We provide a `supportsCssVariables` function within the `util.js` which we recommend using, as it handles this for you. |
 | `isUnbounded() => boolean` | Whether or not the ripple should be considered unbounded. |
 | `isSurfaceActive() => boolean` | Whether or not the surface the ripple is acting upon is [active](https://www.w3.org/TR/css3-selectors/#useraction-pseudos). We use this to detect whether or not a keyboard event has activated the surface the ripple is on. This does not need to make use of `:active` (which is what we do); feel free to supply your own heuristics for it. |
 | `isSurfaceDisabled() => boolean` | Whether or not the ripple is attached to a disabled component. If true, the ripple will not activate. |
@@ -380,6 +379,17 @@ this.ripple_ = new MDCRippleFoundation({
 });
 ```
 
+## Caveat: Edge
+
+> TL;DR ripples are disabled in Edge because of issues with its support of CSS variables in pseudo elements.
+
+Edge introduced CSS variables in version 15. Unfortunately, there are
+[known issues](https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11495448/)
+involving its implementation for pseudo-elements which cause ripples to behave incorrectly.
+We feature-detect Edge's buggy behavior as it pertains to `::before`, and do not initialize ripples if the bug is
+observed. Earlier versions of Edge (and IE) are not affected, as they do not report support for CSS variables at all,
+and as such ripples are never initialized.
+
 ## Caveat: Safari
 
 > TL;DR ripples are disabled in Safari < 10 because of a nasty CSS variables bug.
@@ -416,13 +426,13 @@ to your theme via a custom variable will not propagate to ripples._ We don't see
 
 External frameworks and libraries can use the following utility methods when integrating a component.
 
-#### util.supportsCssVariables(windowObj) => Boolean
+#### util.supportsCssVariables(windowObj, forceRefresh = false) => Boolean
 
-Determine whether the current browser supports CSS variables (custom properties).
+Determine whether the current browser supports CSS variables (custom properties). This function caches its result; `forceRefresh` will force recomputation, but is used mainly for testing and should not be necessary in normal use.
 
 #### util.applyPassive(globalObj = window, forceRefresh = false) => object
 
-Determine whether the current browser supports passive event listeners, and if so, use them.
+Determine whether the current browser supports passive event listeners, and if so, use them. This function caches its result; `forceRefresh` will force recomputation, but is used mainly for testing and should not be necessary in normal use.
 
 #### getMatchesProperty(HTMLElementPrototype) => Function
 

packages/mdc-ripple/util.js

@@ -14,14 +14,54 @@
  * limitations under the License.
  */
 
-/** @private {boolean|undefined} */
+/**
+ * Stores result from supportsCssVariables to avoid redundant processing to detect CSS custom variable support.
+ * @private {boolean|undefined}
+ */
+let supportsCssVariables_;
+
+/**
+ * Stores result from applyPassive to avoid redundant processing to detect passive event listener support.
+ * @private {boolean|undefined}
+ */
 let supportsPassive_;
 
 /**
  * @param {!Window} windowObj
+ * @return {boolean}
+ */
+function detectEdgePseudoVarBug(windowObj) {
+  // Detect versions of Edge with buggy var() support
+  // See: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11495448/
+  const document = windowObj.document;
+  const className = 'test-edge-css-var';
+  const styleNode = document.createElement('style');
+  document.head.appendChild(styleNode);
+  const sheet = styleNode.sheet;
+  // Internet Explorer 11 requires indices to always be specified to insertRule
+  sheet.insertRule(`:root { --${className}: 1px solid #000; }`, 0);
+  sheet.insertRule(`.${className} { visibility: hidden; }`, 1);
+  sheet.insertRule(`.${className}::before { border: var(--${className}); }`, 2);
+  const node = document.createElement('div');
+  node.className = className;
+  document.body.appendChild(node);
+  // Bug exists if ::before style ends up propagating to the parent element
+  const hasPseudoVarBug = windowObj.getComputedStyle(node).borderTopStyle === 'solid';
+  node.remove();
+  styleNode.remove();
+  return hasPseudoVarBug;
+}
+
+/**
+ * @param {!Window} windowObj
+ * @param {boolean=} forceRefresh
  * @return {boolean|undefined}
  */
-export function supportsCssVariables(windowObj) {
+export function supportsCssVariables(windowObj, forceRefresh = false) {
+  if (typeof supportsCssVariables_ === 'boolean' && !forceRefresh) {
+    return supportsCssVariables_;
+  }
+
   const supportsFunctionPresent = windowObj.CSS && typeof windowObj.CSS.supports === 'function';
   if (!supportsFunctionPresent) {
     return;
@@ -34,7 +74,13 @@ export function supportsCssVariables(windowObj) {
     windowObj.CSS.supports('(--css-vars: yes)') &&
     windowObj.CSS.supports('color', '#00000000')
   );
-  return explicitlySupportsCssVars || weAreFeatureDetectingSafari10plus;
+
+  if (explicitlySupportsCssVars || weAreFeatureDetectingSafari10plus) {
+    supportsCssVariables_ = !detectEdgePseudoVarBug(windowObj);
+  } else {
+    supportsCssVariables_ = false;
+  }
+  return supportsCssVariables_;
 }
 
 //

test/unit/mdc-ripple/helpers.js

@@ -47,3 +47,46 @@ export function captureHandlers(adapter) {
   const handlers = baseCaptureHandlers(adapter, 'registerInteractionHandler');
   return handlers;
 }
+
+// Creates a mock window object with all members necessary to test util.supportsCssVariables
+// in cases where window.CSS.supports indicates the feature is supported.
+export function createMockWindowForCssVariables() {
+  const getComputedStyle = td.func('window.getComputedStyle');
+  const remove = () => mockWindow.appendedNodes--;
+  const mockDoc = {
+    body: {
+      appendChild: () => mockWindow.appendedNodes++,
+    },
+    createElement: td.func('document.createElement'),
+    head: {
+      appendChild: () => mockWindow.appendedNodes++,
+    },
+  };
+  const mockSheet = {
+    insertRule: () => {},
+  };
+
+  td.when(getComputedStyle(td.matchers.anything())).thenReturn({
+    borderTopStyle: 'none',
+  });
+
+  td.when(mockDoc.createElement('div')).thenReturn({
+    remove: remove,
+  });
+
+  td.when(mockDoc.createElement('style')).thenReturn({
+    remove: remove,
+    sheet: mockSheet,
+  });
+
+  const mockWindow = {
+    // Expose count of nodes that have been appended and not removed, to be verified in tests
+    appendedNodes: 0,
+    CSS: {
+      supports: td.func('.supports'),
+    },
+    document: mockDoc,
+    getComputedStyle: getComputedStyle,
+  };
+  return mockWindow;
+}

test/unit/mdc-ripple/util.test.js

@@ -15,38 +15,42 @@
  */
 
 import {assert} from 'chai';
+import {createMockWindowForCssVariables} from './helpers';
 import td from 'testdouble';
 import * as util from '../../../packages/mdc-ripple/util';
 
 suite('MDCRipple - util');
 
 test('#supportsCssVariables returns true when CSS.supports() returns true for css vars', () => {
-  const windowObj = {
-    CSS: {
-      supports: td.func('.supports'),
-    },
-  };
+  const windowObj = createMockWindowForCssVariables();
   td.when(windowObj.CSS.supports('--css-vars', td.matchers.anything())).thenReturn(true);
-  assert.isOk(util.supportsCssVariables(windowObj));
+  assert.isOk(util.supportsCssVariables(windowObj, true));
+  assert.equal(windowObj.appendedNodes, 0, 'All nodes created in #supportsCssVariables should be removed');
 });
 
 test('#supportsCssVariables returns true when feature-detecting its way around Safari < 10', () => {
-  const windowObj = {
-    CSS: {
-      supports: td.func('.supports'),
-    },
-  };
+  const windowObj = createMockWindowForCssVariables();
   td.when(windowObj.CSS.supports('--css-vars', td.matchers.anything())).thenReturn(false);
   td.when(windowObj.CSS.supports(td.matchers.contains('(--css-vars:'))).thenReturn(true);
   td.when(windowObj.CSS.supports('color', '#00000000')).thenReturn(true);
-  assert.isOk(util.supportsCssVariables(windowObj), 'true iff both CSS Vars and #rgba are supported');
+  assert.isOk(util.supportsCssVariables(windowObj, true), 'true iff both CSS Vars and #rgba are supported');
 
   td.when(windowObj.CSS.supports(td.matchers.contains('(--css-vars:'))).thenReturn(false);
-  assert.isNotOk(util.supportsCssVariables(windowObj), 'false if CSS Vars are supported but not #rgba');
+  assert.isNotOk(util.supportsCssVariables(windowObj, true), 'false if #rgba is supported but not CSS Vars');
   td.when(windowObj.CSS.supports(td.matchers.contains('(--css-vars:'))).thenReturn(true);
 
   td.when(windowObj.CSS.supports('color', '#00000000')).thenReturn(false);
-  assert.isNotOk(util.supportsCssVariables(windowObj), 'false if #rgba is supported but not CSS Vars');
+  assert.isNotOk(util.supportsCssVariables(windowObj, true), 'false if CSS Vars are supported but not #rgba');
+  assert.equal(windowObj.appendedNodes, 0, 'All nodes created in #supportsCssVariables should be removed');
+});
+
+test('#supportsCssVariables returns false when feature-detecting Edge var() bug with pseudo selectors', () => {
+  const windowObj = createMockWindowForCssVariables();
+  td.when(windowObj.getComputedStyle(td.matchers.anything())).thenReturn({
+    borderTopStyle: 'solid',
+  });
+  assert.isNotOk(util.supportsCssVariables(windowObj, true), 'false if Edge bug is detected');
+  assert.equal(windowObj.appendedNodes, 0, 'All nodes created in #supportsCssVariables should be removed');
 });
 
 test('#supportsCssVariables returns false when CSS.supports() returns false for css vars', () => {
@@ -56,7 +60,7 @@ test('#supportsCssVariables returns false when CSS.supports() returns false for
     },
   };
   td.when(windowObj.CSS.supports('--css-vars', td.matchers.anything())).thenReturn(false);
-  assert.isNotOk(util.supportsCssVariables(windowObj));
+  assert.isNotOk(util.supportsCssVariables(windowObj, true));
 });
 
 test('#supportsCssVariables returns false when CSS.supports is not a function', () => {
@@ -65,14 +69,14 @@ test('#supportsCssVariables returns false when CSS.supports is not a function',
       supports: 'nope',
     },
   };
-  assert.isNotOk(util.supportsCssVariables(windowObj));
+  assert.isNotOk(util.supportsCssVariables(windowObj, true));
 });
 
 test('#supportsCssVariables returns false when CSS is not an object', () => {
   const windowObj = {
     CSS: null,
   };
-  assert.isNotOk(util.supportsCssVariables(windowObj));
+  assert.isNotOk(util.supportsCssVariables(windowObj, true));
 });
 
 test('applyPassive returns an options object for browsers that support passive event listeners', () => {

test/unit/mdc-tabs/mdc-tab.test.js

@@ -44,24 +44,30 @@ test('attachTo returns a component instance', () => {
   assert.isOk(MDCTab.attachTo(getFixture()) instanceof MDCTab);
 });
 
-if (supportsCssVariables(window)) {
-  test('#constructor initializes the root element with a ripple', () => {
-    const raf = createMockRaf();
-    const {root} = setupTest();
-    raf.flush();
-    assert.isOk(root.classList.contains('mdc-ripple-upgraded'));
-    raf.restore();
-  });
-
-  test('#destroy cleans up ripple on tab', () => {
-    const raf = createMockRaf();
-    const {root, component} = setupTest();
-    raf.flush();
-    component.destroy();
-    raf.flush();
-    assert.isNotOk(root.classList.contains('mdc-ripple-upgraded'));
-  });
-}
+test('#constructor initializes the root element with a ripple in browsers that support it', function() {
+  if (!supportsCssVariables(window, true)) {
+    this.skip(); // eslint-disable-line no-invalid-this
+    return;
+  }
+  const raf = createMockRaf();
+  const {root} = setupTest();
+  raf.flush();
+  assert.isOk(root.classList.contains('mdc-ripple-upgraded'));
+  raf.restore();
+});
+
+test('#destroy cleans up tab\'s ripple in browsers that support it', function() {
+  if (!supportsCssVariables(window, true)) {
+    this.skip(); // eslint-disable-line no-invalid-this
+    return;
+  }
+  const raf = createMockRaf();
+  const {root, component} = setupTest();
+  raf.flush();
+  component.destroy();
+  raf.flush();
+  assert.isNotOk(root.classList.contains('mdc-ripple-upgraded'));
+});
 
 test('#get computedWidth returns computed width of tab', () => {
   const {root, component} = setupTest();