std-switch: Fix no transition in a page with text selection

'transitionrun' event is delayed if a page has text selection, and the
transition detection logic in style.mjs didn't work in this case.

After this CL, we remove '$part-transitioning' part in
attributeChangedCallback. This works well because Blink doesn't compute
style for <std-switch> before calling attributeChangedCallback for
'on' attribute.

Change-Id: I8f25eca61fd52441d06bf86173c19d1c764bb859
Bug: 992454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758002Reviewed-by: Fergal Daly <fergal@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688427}

third_party/blink/renderer/core/script/resources/layered_api/elements/switch/index.mjs

@@ -28,6 +28,8 @@ export class StdSwitchElement extends HTMLElement {
     return [STATE_ATTR];
   }
 
+  #inUserAction = false;
+
   constructor() {
     super();
     if (new.target !== StdSwitchElement) {
@@ -47,6 +49,11 @@ export class StdSwitchElement extends HTMLElement {
       // TODO(tkent): We should not add aria-checked attribute.
       // https://github.com/WICG/aom/issues/127
       this.setAttribute('aria-checked', newValue !== null ? 'true' : 'false');
+      if (!this.#inUserAction) {
+        for (const element of this[_containerElement].querySelectorAll('*')) {
+          style.unmarkTransition(element);
+        }
+      }
     }
   }
 
@@ -91,7 +98,12 @@ export class StdSwitchElement extends HTMLElement {
     for (const element of this[_containerElement].querySelectorAll('*')) {
       style.markTransition(element);
     }
-    this.on = !this.on;
+    this.#inUserAction = true;
+    try {
+      this.on = !this.on;
+    } finally {
+      this.#inUserAction = false;
+    }
     this.dispatchEvent(new Event('input', {bubbles: true}));
     this.dispatchEvent(new Event('change', {bubbles: true}));
   }

third_party/blink/renderer/core/script/resources/layered_api/elements/switch/style.mjs

@@ -152,74 +152,33 @@ export function styleSheetFactory() {
 }
 
 /**
+ * Add '$part-transitioning' part to the element if the element already has
+ * a part name.
+ *
+ * TODO(tkent): We should apply custom state.
+ *
  * @param {!Element} element
  */
-function setupTransitionCounter(element) {
-  if (element.runningTransitions !== undefined) {
+export function markTransition(element) {
+  // Should check hasAttribute() to avoid creating a DOMTokenList instance.
+  if (!element.hasAttribute('part') || element.part.length < 1) {
     return;
   }
-  element.runningTransitions = 0;
-  element.addEventListener('transitionrun', e => {
-    if (e.target === element) {
-      ++element.runningTransitions;
-    }
-  });
-  function handleEndOrCancel(e) {
-    // Need to check runningTransitions>0 due to superfluous transitioncancel
-    // events; crbug.com/979556.
-    if (e.target === element && element.runningTransitions > 0) {
-      --element.runningTransitions;
-    }
-  }
-  element.addEventListener('transitionend', handleEndOrCancel);
-  element.addEventListener('transitioncancel', handleEndOrCancel);
+  element.part.add(element.part[0] + '-transitioning');
 }
 
 /**
- * Add '$part-transitioning' part to the element, and remove it on
- * 'transitionend' event or remove it immediately if the element has no
- * transitions.
+ * Remove '$part-transitioning' part from the element if the element already has
+ * a part name.
  *
  * TODO(tkent): We should apply custom state.
  *
  * @param {!Element} element
  */
-export function markTransition(element) {
+export function unmarkTransition(element) {
   // Should check hasAttribute() to avoid creating a DOMTokenList instance.
   if (!element.hasAttribute('part') || element.part.length < 1) {
     return;
   }
-
-  setupTransitionCounter(element);
-
-  const partName = element.part[0] + '-transitioning';
-  if (element.part.contains(partName)) {
-    return;
-  }
-  // The style with partName might have transitions, and might have no
-  // transitions. We need to add partName anyway because we have no other
-  // ways to check existence of transitions.  Then, we need to remove
-  // partName because state change without markTransition() should have
-  // style without partName.
-  //
-  // We add partName in an animation frame, and continue to request
-  // animation frames until runningTransitions becomes 0. If the style with
-  // partName has no transitions, runningTransitions keeps 0, and the second
-  // animation frame removes partName.
-  window.requestAnimationFrame(() => {
-    element.part.add(partName);
-
-    // If the element has a transition, it must start on the rendering just
-    // after this rAF callback. So we check runningTransitions in the next
-    // frame.
-    function removeIfNoTransitions() {
-      // No transitions started, or all transitions were completed.
-      if (element.runningTransitions === 0) {
-        element.part.remove(partName);
-      } else {
-        window.requestAnimationFrame(removeIfNoTransitions);
-      }
-    }
-    window.requestAnimationFrame(removeIfNoTransitions);
-  });
+  element.part.remove(element.part[0] + '-transitioning');
 }

third_party/blink/web_tests/std-switch/click-transition.html

@@ -4,6 +4,7 @@
 <script src="../resources/testdriver.js"></script>
 <script src="../resources/testdriver-vendor.js"></script>
 <body>
+<p>Some text</p>
 <script type="module">
 import 'std:elements/switch';
 
@@ -13,39 +14,71 @@ function singleFrame() {
   });
 }
 
-promise_test(async () => {
-  let switchElement = document.createElement('std-switch');
-  document.body.appendChild(switchElement);
-  assert_false(switchElement.on);
-  let thumb = internals.shadowRoot(switchElement).getElementById('thumb');
-  let transitionStartCounter = 0;
-  let transitionEndCounter = 0;
-  thumb.addEventListener('transitionstart', e => { ++transitionStartCounter; });
-  thumb.addEventListener('transitionend', e => { ++transitionEndCounter; });
-
-  await test_driver.click(switchElement);
-  assert_true(switchElement.on, 'click');
+function setupSwitchElement() {
+  let element = document.createElement('std-switch');
+  document.body.appendChild(element);
+
+  let thumb = internals.shadowRoot(element).getElementById('thumb');
+  element.transitionStartCounter = 0;
+  element.transitionEndCounter = 0;
+  thumb.addEventListener('transitionstart', e => {
+    ++element.transitionStartCounter;
+  });
+  thumb.addEventListener('transitionend', e => {
+    ++element.transitionEndCounter;
+  });
+  return element;
+}
+
+async function assertClickTransition(element) {
+  assert_false(element.on);
+  await test_driver.click(element);
+  assert_true(element.on, 'click');
   await singleFrame();
-  assert_true(transitionStartCounter > 0, 'A transition starts');
-  if (transitionStartCounter > transitionEndCounter) {
+  assert_true(element.transitionStartCounter > 0, 'A transition starts');
+  if (element.transitionStartCounter > element.transitionEndCounter) {
     await new Promise((resolve, reject) => {
+      let thumb = internals.shadowRoot(element).getElementById('thumb');
       thumb.addEventListener('transitionend', () => {
-        if (transitionStartCounter == transitionEndCounter) {
+        if (element.transitionStartCounter == element.transitionEndCounter) {
           resolve();
         }
       });
     });
   }
-  assert_equals(thumb.runningTransitions, 0);
-  // The track element doesn't animate by default.
-  let trackElement = internals.shadowRoot(switchElement).getElementById('track');
-  assert_equals(trackElement.runningTransitions, 0);
+}
 
-  // Changing the state with an IDL attribute should not animate.
-  transitionStartCounter = 0;
-  switchElement.on = false;
+// This should be called just after an operation which should not start
+// transition. element.transitionStartCounter must be 0 before the operation.
+async function assertNoTransition(element) {
   await singleFrame();
-  assert_equals(transitionStartCounter, 0, 'No additional events');
+  assert_equals(element.transitionStartCounter, 0, 'No transitionstart events');
+}
+
+promise_test(async () => {
+  let switchElement = setupSwitchElement();
+  await assertClickTransition(switchElement);
+
+  // Changing the state with an IDL attribute should not animate even after
+  // click transition.
+  switchElement.transitionStartCounter = 0;
+  switchElement.on = false;
+  await assertNoTransition(switchElement);
 }, 'Click event handler should start transition');
+
+promise_test(async () => {
+  let switchElement = setupSwitchElement();
+  switchElement.on = !switchElement.on;
+  await assertNoTransition(switchElement);
+}, 'Changing the state with an IDL attribute should not start transition');
+
+promise_test(async () => {
+  // crbug.com/992454 Transition didn't start if there is a text selection.
+  const range = document.createRange();
+  range.selectNodeContents(document.querySelector('p'));
+  window.getSelection().addRange(range);
+
+  await assertClickTransition(setupSwitchElement());
+}, 'Click event handler should start transition even with text selection');
 </script>
 </body>