fix(ripple): Re-flow logic to avoid crashing Edge

material-components · Apr 9, 2018 · 7d25012 · 7d25012
1 parent 8c4fbdc
commit 7d25012
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 15 deletions.
diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js
@@ -330,25 +330,41 @@ class MDCRippleFoundation extends MDCFoundation {
       this.registerDeactivationHandlers_(e);
     }
 
-    requestAnimationFrame(() => {
-      // This needs to be wrapped in an rAF call b/c web browsers
-      // report active states inconsistently when they're called within
-      // event handling code:
-      // - https://bugs.chromium.org/p/chromium/issues/detail?id=635971
-      // - https://bugzilla.mozilla.org/show_bug.cgi?id=1293741
-      activationState.wasElementMadeActive = (e && e.type === 'keydown') ? this.adapter_.isSurfaceActive() : true;
-      if (activationState.wasElementMadeActive) {
-        this.animateActivation_();
-      } else {
-        // Reset activation state immediately if element was not made active.
-        this.activationState_ = this.defaultActivationState_();
-      }
+    activationState.wasElementMadeActive = this.checkElementMadeActive_(e);
+    if (activationState.wasElementMadeActive) {
+      this.animateActivation_();
+    }
 
+    requestAnimationFrame(() => {
       // Reset array on next frame after the current event has had a chance to bubble to prevent ancestor ripples
       activatedTargets = [];
+
+      if (!activationState.wasElementMadeActive && e && e.type === 'keydown' && (e.key === ' ' || e.keyCode === 32)) {
+        // If space was pressed, try again within an rAF call to detect :active, because different UAs report
+        // active states inconsistently when they're called within event handling code:
+        // - https://bugs.chromium.org/p/chromium/issues/detail?id=635971
+        // - https://bugzilla.mozilla.org/show_bug.cgi?id=1293741
+        // We try first outside rAF to support Edge, which does not exhibit this problem, but will crash if a CSS
+        // variable is set within a rAF callback for a submit button interaction (#2241).
+        activationState.wasElementMadeActive = this.checkElementMadeActive_(e);
+        if (activationState.wasElementMadeActive) {
+          this.animateActivation_();
+        } else {
+          // Reset activation state immediately if element was not made active.
+          this.activationState_ = this.defaultActivationState_();
+        }
+      }
     });
   }
 
+  /**
+   * @param {?Event} e
+   * @private
+   */
+  checkElementMadeActive_(e) {
+    return (e && e.type === 'keydown') ? this.adapter_.isSurfaceActive() : true;
+  }
+
   /**
    * @param {?Event=} event Optional event containing position information.
    */

diff --git a/test/unit/mdc-ripple/foundation-activation.test.js b/test/unit/mdc-ripple/foundation-activation.test.js
@@ -176,17 +176,42 @@ testFoundation('sets FG position from the coords to the center within surface on
     ));
   });
 
-testFoundation('adds activation classes on keydown when surface is made active',
+testFoundation('adds activation classes on keydown when surface is made active on same frame',
   ({foundation, adapter, mockRaf}) => {
     const handlers = captureHandlers(adapter, 'registerInteractionHandler');
     td.when(adapter.isSurfaceActive()).thenReturn(true);
     foundation.init();
     mockRaf.flush();
 
     handlers.keydown();
+    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION));
+  });
+
+testFoundation('adds activation classes on keydown when surface only reflects :active on next frame for space keydown',
+  ({foundation, adapter, mockRaf}) => {
+    const handlers = captureHandlers(adapter, 'registerInteractionHandler');
+    td.when(adapter.isSurfaceActive()).thenReturn(false, true);
+    foundation.init();
     mockRaf.flush();
 
-    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION));
+    handlers.keydown({key: ' '});
+    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION), {times: 0});
+
+    mockRaf.flush();
+    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION), {times: 1});
+  });
+
+testFoundation('does not add activation classes on keydown when surface is not made active',
+  ({foundation, adapter, mockRaf}) => {
+    const handlers = captureHandlers(adapter, 'registerInteractionHandler');
+    td.when(adapter.isSurfaceActive()).thenReturn(false, false);
+    foundation.init();
+    mockRaf.flush();
+
+    handlers.keydown({key: ' '});
+    mockRaf.flush();
+
+    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION), {times: 0});
   });
 
 testFoundation('sets FG position to center on non-pointer activation', ({foundation, adapter, mockRaf}) => {