Media Controls: Prevent overflow from closing on a click and drag

This CL adds a check in the popup menu click handling to only assume an item was selected when the click target is not the popup menu itself. This fixes an issue where the popup would close when a user clicked and dragged between two items (since this sends a "click" event with the parent as the target). Bug: 934225 Change-Id: I67b7079b0a8a0d1d6405953808f805cfed0c83ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632421Reviewed-by: Jazz Xu <jazzhsu@chromium.org> Commit-Queue: Tommy Steimel <steimel@chromium.org> Cr-Commit-Position: refs/heads/master@{#663846}

Media Controls: Prevent overflow from closing on a click and drag
This CL adds a check in the popup menu click handling to only assume an item was selected when the click target is not the popup menu itself. This fixes an issue where the popup would close when a user clicked and dragged between two items (since this sends a "click" event with the parent as the target). Bug: 934225 Change-Id: I67b7079b0a8a0d1d6405953808f805cfed0c83ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632421Reviewed-by: Jazz Xu <jazzhsu@chromium.org> Commit-Queue: Tommy Steimel <steimel@chromium.org> Cr-Commit-Position: refs/heads/master@{#663846}
900c5542 · Tommy Steimel · Commit Bot · fac88d7f · 900c5542 · 900c5542
Commit 900c5542 authored May 28, 2019 by Tommy Steimel Committed by Commit Bot May 28, 2019
2 changed files
--- a/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.cc
+++ b/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.cc
@@ -136,7 +136,10 @@ void MediaControlPopupMenuElement::DefaultEventHandler(Event& event) {
        ->PostTask(FROM_HERE,
                   WTF::Bind(&MediaControlPopupMenuElement::HideIfNotFocused,
                             WrapWeakPersistent(this)));
-  } else if (event.type() == event_type_names::kClick) {
+  } else if (event.type() == event_type_names::kClick &&
+             event.target() != this) {
+    // Since event.target() != this, we know that one of our children was
+    // clicked.
    OnItemSelected();

    event.stopPropagation();

--- a/third_party/blink/web_tests/media/controls/click-and-drag-on-overflow-menu-doesnt-close.html
+++ b/third_party/blink/web_tests/media/controls/click-and-drag-on-overflow-menu-doesnt-close.html
+<!DOCTYPE html>
+<html>
+<title>Test that when a user clicks and drags on the overflow it does not disappear.</title>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../media-controls.js"></script>
+<video controls width=500 preload=none src="https://need_a_url_here_to.show/download/button.mp4"></video>
+<script>
+async_test(t => {
+  const video = document.querySelector('video');
+  const menu = overflowMenu(video);
+  const overflowBtn = overflowButton(video);
+  const downloadBtn = downloadsOverflowItem(video);
+  const captionsBtn = captionsOverflowItem(video);
+
+  // Add text tracks so that the captions option appears.
+  [ '../track/captions-webvtt/captions-fast.vtt',
+    '../track/captions-webvtt/captions-rtl.vtt' ].forEach(source => {
+    const track = document.createElement('track');
+    track.src = source;
+    track.kind = 'captions';
+    video.appendChild(track);
+  });
+
+  enableTestMode(video);
+
+  window.addEventListener('load', t.step_func(() => {
+    // Open the overflow menu and click and drag on it.
+    singleTapOnControl(overflowBtn, t.step_func(clickAndDrag));
+  }), { once: true });
+
+  // Neither the downloads button nor the captions button should receive a click during the click and drag.
+  downloadBtn.addEventListener('click', t.unreached_func('Downloads received a click'));
+  captionsBtn.addEventListener('click', t.unreached_func('Captions received a click'));
+
+  function clickAndDrag() {
+    assert_true(isControlVisible(menu), 'Overflow menu should be visible before we start click-and-drag');
+    assert_true(isControlVisible(downloadBtn), 'Download button should be visible');
+    assert_true(isControlVisible(captionsBtn), 'Captions button should be visible');
+
+    const downloadPosition = elementCoordinates(downloadBtn);
+    const captionsPosition = elementCoordinates(captionsBtn);
+
+    // Click down on downloads but drag and release over captions.
+    chrome.gpuBenchmarking.pointerActionSequence([{
+      source: 'mouse',
+      actions: [
+        { name: 'pointerDown', x: downloadPosition[0], y: downloadPosition[1] },
+        { name: 'pointerMove', x: captionsPosition[0], y: captionsPosition[1] },
+        { name: 'pointerUp', x: captionsPosition[0], y: captionsPosition[1] },
+      ]
+    }], t.step_func_done(() => {
+      assert_true(isControlVisible(menu), 'Overflow menu should still be visible after click-and-drag');
+    }));
+  }
+});
+</script>
+</html>