From e0c4d92eb81dd58238a0ea5a8c0ebe8104fd9546 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Mon, 9 Apr 2018 15:52:59 -0400 Subject: [PATCH 1/2] fix(ripple): Re-flow logic to avoid crashing Edge --- packages/mdc-ripple/foundation.js | 42 +++++++++++++------ .../mdc-ripple/foundation-activation.test.js | 29 ++++++++++++- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js index 53647190473..10d13788214 100644 --- a/packages/mdc-ripple/foundation.js +++ b/packages/mdc-ripple/foundation.js @@ -330,25 +330,43 @@ class MDCRippleFoundation extends MDCFoundation { this.registerDeactivationHandlers_(e); } + activationState.wasElementMadeActive = this.checkElementMadeActive_(e); + if (activationState.wasElementMadeActive) { + this.animateActivation_(); + } + 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 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_(); + } + } + + if (!activationState.wasElementMadeActive) { // Reset activation state immediately if element was not made active. this.activationState_ = this.defaultActivationState_(); } - - // Reset array on next frame after the current event has had a chance to bubble to prevent ancestor ripples - activatedTargets = []; }); } + /** + * @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 index 200eba46d1d..9af5064c0dd 100644 --- a/test/unit/mdc-ripple/foundation-activation.test.js +++ b/test/unit/mdc-ripple/foundation-activation.test.js @@ -176,7 +176,7 @@ 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); @@ -184,9 +184,34 @@ testFoundation('adds activation classes on keydown when surface is made active', 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}) => { From b7c71ecb34a68dd6cb87072740d9347f6b7899d3 Mon Sep 17 00:00:00 2001 From: "Kenneth G. Franqueiro" Date: Tue, 10 Apr 2018 15:42:38 -0400 Subject: [PATCH 2/2] WIP Remove redundant conditions --- packages/mdc-ripple/foundation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mdc-ripple/foundation.js b/packages/mdc-ripple/foundation.js index 10d13788214..60045c8dc2e 100644 --- a/packages/mdc-ripple/foundation.js +++ b/packages/mdc-ripple/foundation.js @@ -339,7 +339,7 @@ class MDCRippleFoundation extends MDCFoundation { // 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 (!activationState.wasElementMadeActive && (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