Create MediaControls only when native controls are used |
||
Issue descriptionThis CL is about lazy initialising the media controls in Blink.
,
Jun 8 2017
Beware that the overlay Cast button needs to be initialized when 'controls' is not set (controls need to attach a listener to get the availability and so on)
,
Jun 8 2017
This *exactly* why I'm CC'ing you :) Would you be interested to move the overlay to the MediaRemoting layer which isn't part of the media_controls. It would create a bit of repetition but for now, I will have to land a CL that will only enable this on desktop.
,
Jun 8 2017
would the overlay play button move too? I don't recall if we show it when controls is not present. I do want various overlays to be aware of each other. The remoting one does overlap with the play one and also shows the duplicate cast icon when overlay cast button is shown. Seems like we need to separate the lifecycles of media controls panel and the overlay panel: the former is bound to the 'controls' attribute while the latter is bound to the media element.
,
Jun 8 2017
We don't show the overlay play button if native controls are not meant to be used. I agree that in general we might want to have a shadow dom for overlays but so far, the only real non-native controls overlay we have are related to remoting.
,
Jul 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fcc3b11bbc8233e2f338a89ef7245c372dc58115 commit fcc3b11bbc8233e2f338a89ef7245c372dc58115 Author: Mounir Lamouri <mlamouri@chromium.org> Date: Sat Jul 15 14:18:50 2017 Lazy initialize media controls when not using the Cast overlay. The work to abstract the media controls from the HTMLMediaElement allows us to do this without risks. However, when the Cast overlay has to be used, the controls need to be created even if they are not used. The code implementing this logic is part of the native controls so we can't lazy initialise the controls the same way. Bug: 731143 Change-Id: I2eb912ab6124c24000860007c1d6e6aa3300c440 Reviewed-on: https://chromium-review.googlesource.com/528176 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Anton Vayvod <avayvod@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/heads/master@{#486992} [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/content/child/runtime_features.cc [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/content/public/common/content_features.cc [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/content/public/common/content_features.h [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/third_party/WebKit/Source/platform/exported/WebRuntimeFeatures.cpp [modify] https://crrev.com/fcc3b11bbc8233e2f338a89ef7245c372dc58115/third_party/WebKit/public/platform/WebRuntimeFeatures.h
,
Mar 28 2018
Is there any more work needed for this?
,
Mar 28 2018
We can probably fix even though we could one day implement it on Android. |
||
►
Sign in to add a comment |
||
Comment 1 by mlamouri@chromium.org
, Jun 8 2017Labels: -M-60 M-61