New issue
Advanced search Search tips

Issue 821834 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug


Participants' hotlists:
Modern-Media-Controls


Sign in to add a comment

Media Controls: double tap and double click behaviour shouldn't be OS based

Project Member Reported by mlamouri@chromium.org, Mar 14 2018

Issue description

Following François' post on G+, someone asked if double tap could work on Chrome OS with touch instead of double click to fullscreen. Could we use a different logic depending on tap/click instead of a flag? I can see how that could be an issue for the tests though :)
 
I don't think it should be an issue for tests, since we can fake both touch and non-touch events. Is that going to be confusing for users on touch+mouse devices where the double-tap is doing something different than the double-click? Also, we'll need to decide how/when to show the scrubbing message, which currently uses the flag to decide whether or not to be shown
I think the scrubbing message could be shown when the user scrubs with a touch gesture instead of a mouse one. When we call `BeginScrubbing()`, we could pass a boolean that gives the origin. WDYT?
Cc: steimel@chromium.org
Labels: -Pri-3 Pri-2
Owner: ----
Status: Available (was: Unconfirmed)
Summary: Media Controls: double tap and double click behaviour shouldn't be OS based (was: Media Controls: double tap and double check should have a direct )
Owner: steimel@chromium.org
Status: Started (was: Available)
Labels: -Pri-2 Pri-3
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46546d34e5c8ca162b7de0b37e42460b56ef8a7e

commit 46546d34e5c8ca162b7de0b37e42460b56ef8a7e
Author: Tommy Steimel <steimel@chromium.org>
Date: Thu Mar 29 14:52:31 2018

DoubleTapToJump: Depend on touch events instead of RuntimeEnabledFeature

This CL removes the DoubleTapToJump RuntimeEnabledFeature that was used
to determine whether to jump a video 10 seconds or go into fullscreen
on a double-tap. Instead, the video will jump 10 seconds if both taps
are from touch events.

Bug:  821834 
Change-Id: Ie0fa6a56c3775a74b575ae2ac7343afcf47a8d47
Reviewed-on: https://chromium-review.googlesource.com/981417
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546831}
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/content/child/runtime_features.cc
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/doubletap-to-jump-backwards-at-start.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/doubletap-to-jump-backwards.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/doubletap-to-jump-forwards-too-short.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/doubletap-to-jump-forwards.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/scrubbing-touch.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/controls/modern/scrubbing.html
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/LayoutTests/media/media-controls.js
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/modules/media_controls/elements/MediaControlOverlayPlayButtonElement.cpp
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/modules/media_controls/elements/MediaControlTimelineElement.cpp
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/platform/exported/WebRuntimeFeatures.cpp
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/Source/platform/runtime_enabled_features.json5
[modify] https://crrev.com/46546d34e5c8ca162b7de0b37e42460b56ef8a7e/third_party/WebKit/public/platform/WebRuntimeFeatures.h

steimel@, is this ready to be fixed?
Status: Fixed (was: Started)

Sign in to add a comment