preload=none video doesn't look great |
||||||||||||
Issue descriptionGoogle Chrome 67.0.3361.0 (Official Build) canary (64-bit) What steps will reproduce the problem? (1) Enable modern media controls flag (2) Open a new tab to data:text/html,<html><video controls src="https://storage.googleapis.com/media-session/caminandes/short.mp4" preload="none"> What is the expected result? The video player should look great. What happens instead? The video player gradient doesn't look that great ;(
,
Mar 5 2018
I just checked YouTube and the video player gradient is indeed an image URL: data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAADGCAYAAAAT+OqFAAAAdklEQVQoz42QQQ7AIAgEF/T/D+kbq/RWAlnQyyazA4aoAB4FsBSA/bFjuF1EOL7VbrIrBuusmrt4ZZORfb6ehbWdnRHEIiITaEUKa5EJqUakRSaEYBJSCY2dEstQY7AuxahwXFrvZmWl2rh4JZ07z9dLtesfNj5q0FU3A5ObbwAAAABJRU5ErkJggg==
,
Mar 9 2018
Assigning to amyroberts@ for input. A few solutions: - hide the scrim when preload=none as we don't quite need it - use an image (but then we would need to handle top/bottom scrim) - find technical solution to make the scrim nicer on a flat background (maybe more detailed gradient if possible?) - show scrim as fixed height with a MAX set depending on <video> height, ie. avoid scrim going all the way to 50% of the video height, making it big and not fine grained enough
,
Mar 9 2018
I looked more into this with fbeaufort@ and I would suggest to do: 1. use an image for the gradient (probably the same as YT but split it in two for top and bottom scrims) 2. hide the scrim when we don't show any UI at the bottom -- I'm not entirely sure if the updated design requires to always show the timeline with preload=none. Reason that it seems that we will not be able to get the CSS gradient to be smooth enough and the different zones will always be visible.
,
Mar 28 2018
,
Apr 3 2018
preload variations from 3/22 work session mocked up here: https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZZIx9p6vbtpx/files/MCHUhUabeme9PLvIDwUyBjwv
,
Apr 3 2018
beccahughes@ has been looking at the scrubber changes.
,
Apr 3 2018
Going through the mocks these are the following changes that are different from what we have implemented so far: Video (preloaded) - Implemented Video (poster) - Show time indicators and playback head Video (no preloading) - Hide time remaining and change poster color Video (no source) - show all controls and add overlay icon Audio (no source) - show disabled controls @amy - the audio controls on the mocks have no grey background. Is this something that is being removed?
,
Apr 4 2018
It looks great! I wonder if these mocks handle properly though the video background color. See Issue 814727.
,
Apr 4 2018
We will unlikely fix the background colour issue for M67 given the short time we have left. I would expect these things to be orthogonal and Tommy mentioned that fixing the background colour wouldn't be trivial.
,
Apr 6 2018
beccahughes@ re: Comment 8, the audio controls have a white background at 90% opacity (like the play/pause button) More details here https://docs.google.com/presentation/d/1Xwu6nvCoxEIjuIdGYt9DRPBHqEX3QQkpQluHcJazCsE/edit#slide=id.g348eb293e9_0_21
,
Apr 11 2018
These are looking good. Can we double check the background color for no source/no preloading? They look just a little light. Spec attached. (should be gradient: 80% #000000 to 100% #000000)
,
Apr 11 2018
Fixed :)
,
Apr 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/096153b99de9f3a02c3a6fef3dbb43a6c701f24f commit 096153b99de9f3a02c3a6fef3dbb43a6c701f24f Author: Becca Hughes <beccahughes@chromium.org> Date: Fri Apr 13 03:35:12 2018 Media Controls: Update preload=none state Update preload=none state based on the new mocks. Also adds ShouldShow[Audio/Video]Controls to MediaControlsImpl to simplify the logic around which set of controls we should show. BUG= 818659 No-Try: true Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ide3bed15161aa5e1e1096275212c718eacb24a77 Reviewed-on: https://chromium-review.googlesource.com/993757 Commit-Queue: Becca Hughes <beccahughes@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Tommy Steimel <steimel@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#550487} [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs [add] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/WebKit/LayoutTests/media/controls/modern/overflow-button-disabled-no-source.html [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_button_element.cc [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [add] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/resources/ic_no_source.svg [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css
,
Apr 13 2018
,
Apr 13 2018
,
Apr 14 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 15 2018
Pls merge your change to M67 branch 3396 by 1:00 PM PT Monday (04/16/18) so we can pick it up for next M67 dev release. Thank you.
,
Apr 16 2018
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2 commit 883aaf56b42d4123e4b862d56f2eb2f0b069c3b2 Author: Becca Hughes <beccahughes@chromium.org> Date: Mon Apr 16 16:12:05 2018 Media Controls: Update preload=none state Update preload=none state based on the new mocks. Also adds ShouldShow[Audio/Video]Controls to MediaControlsImpl to simplify the logic around which set of controls we should show. BUG= 818659 No-Try: true Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ide3bed15161aa5e1e1096275212c718eacb24a77 Reviewed-on: https://chromium-review.googlesource.com/993757 Commit-Queue: Becca Hughes <beccahughes@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Tommy Steimel <steimel@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#550487}(cherry picked from commit 096153b99de9f3a02c3a6fef3dbb43a6c701f24f) Reviewed-on: https://chromium-review.googlesource.com/1014181 Reviewed-by: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#15} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs [add] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/WebKit/LayoutTests/media/controls/modern/overflow-button-disabled-no-source.html [modify] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_button_element.cc [modify] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [add] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/blink/renderer/modules/media_controls/resources/ic_no_source.svg [modify] https://crrev.com/883aaf56b42d4123e4b862d56f2eb2f0b069c3b2/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/096153b99de9f3a02c3a6fef3dbb43a6c701f24f commit 096153b99de9f3a02c3a6fef3dbb43a6c701f24f Author: Becca Hughes <beccahughes@chromium.org> Date: Fri Apr 13 03:35:12 2018 Media Controls: Update preload=none state Update preload=none state based on the new mocks. Also adds ShouldShow[Audio/Video]Controls to MediaControlsImpl to simplify the logic around which set of controls we should show. BUG= 818659 No-Try: true Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ide3bed15161aa5e1e1096275212c718eacb24a77 Reviewed-on: https://chromium-review.googlesource.com/993757 Commit-Queue: Becca Hughes <beccahughes@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Reviewed-by: Tommy Steimel <steimel@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#550487} [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs [add] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/WebKit/LayoutTests/media/controls/modern/overflow-button-disabled-no-source.html [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_button_element.cc [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [add] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/resources/ic_no_source.svg [modify] https://crrev.com/096153b99de9f3a02c3a6fef3dbb43a6c701f24f/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by fbeaufort@chromium.org
, Mar 5 2018