Picture-in-Picture context menu should show for mediastream <video> |
||||||||
Issue descriptionChrome Version : 72.0.3592.0 OS Version: 11196.0.0 What steps will reproduce the problem? 1. Go to https://beaufortfrancois.github.io/sandbox/media/picture-in-picture-get-user-media.html 2. Accept camera prompt 3. Right click video What is the expected result? I should see "Picture in picture" What happens instead of that? "Picture in picture" option is not present. I believe it is because we're only presenting context menu media actions if src is not empty. In that case, we should check for srcObject. See https://chromium.googlesource.com/chromium/src/+/49fe04d0a39a638f31706cb28f939e6682d2f354/third_party/blink/renderer/core/page/context_menu_controller.cc#250
,
Oct 30
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c20223cc6a70cae053ccf5673d8c10913d926453 commit c20223cc6a70cae053ccf5673d8c10913d926453 Author: François Beaufort <beaufort.francois@gmail.com> Date: Thu Nov 08 18:20:46 2018 Add video context menu items to MediaStream video. This CL makes sure video context menu items show for MediaStream video. Only "Picture-in-Picture" and "Show Controls" are enabled for now. This also disables the "Loop" menu if video is a MediaStream or has an infinite duration (Live). Screenshot: https://i.imgur.com/P1t9urA.png Bug: 899466 Change-Id: I309d7cf5918e8c17a84c3adfec4a8d7542336554 Reviewed-on: https://chromium-review.googlesource.com/c/1306997 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/heads/master@{#606543} [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/chrome/browser/renderer_context_menu/render_view_context_menu.cc [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/public/web/web_context_menu_data.h [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/html/media/html_media_element.cc [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/html/media/html_media_element.h [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/layout/hit_test_result.cc [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/layout/hit_test_result.h [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/page/context_menu_controller.cc [modify] https://crrev.com/c20223cc6a70cae053ccf5673d8c10913d926453/third_party/blink/renderer/core/page/context_menu_controller_test.cc
,
Nov 9
Verified in Chromium 72.0.36.07
,
Nov 9
,
Nov 9
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 9
How safe and critical is this change to merge to M71 beta this late in release cycle?
,
Nov 9
This change is simply about adding video context menu items to MediaStream video. Picture-in-Picture for MediaStream video is shipping in M71, hence why this change would be appreciated in M71.
,
Nov 9
Approving merge to M71 branch 3578 based on comment #4 and #8. Please merge ASAP. Thank you.
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1986bf2a6fe739677135510ac1b1c4ed570702b0 commit 1986bf2a6fe739677135510ac1b1c4ed570702b0 Author: François Beaufort <beaufort.francois@gmail.com> Date: Fri Nov 09 21:21:50 2018 Add video context menu items to MediaStream video. This CL makes sure video context menu items show for MediaStream video. Only "Picture-in-Picture" and "Show Controls" are enabled for now. This also disables the "Loop" menu if video is a MediaStream or has an infinite duration (Live). Screenshot: https://i.imgur.com/P1t9urA.png Bug: 899466 Change-Id: I309d7cf5918e8c17a84c3adfec4a8d7542336554 Reviewed-on: https://chromium-review.googlesource.com/c/1306997 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Original-Commit-Position: refs/heads/master@{#606543}(cherry picked from commit c20223cc6a70cae053ccf5673d8c10913d926453) Reviewed-on: https://chromium-review.googlesource.com/c/1329929 Reviewed-by: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/branch-heads/3578@{#620} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/chrome/browser/renderer_context_menu/render_view_context_menu.cc [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/public/web/web_context_menu_data.h [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/html/media/html_media_element.cc [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/html/media/html_media_element.h [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/layout/hit_test_result.cc [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/layout/hit_test_result.h [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/page/context_menu_controller.cc [modify] https://crrev.com/1986bf2a6fe739677135510ac1b1c4ed570702b0/third_party/blink/renderer/core/page/context_menu_controller_test.cc
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1986bf2a6fe739677135510ac1b1c4ed570702b0 Commit: 1986bf2a6fe739677135510ac1b1c4ed570702b0 Author: beaufort.francois@gmail.com Commiter: beaufort.francois@gmail.com Date: 2018-11-09 21:21:50 +0000 UTC Add video context menu items to MediaStream video. This CL makes sure video context menu items show for MediaStream video. Only "Picture-in-Picture" and "Show Controls" are enabled for now. This also disables the "Loop" menu if video is a MediaStream or has an infinite duration (Live). Screenshot: https://i.imgur.com/P1t9urA.png Bug: 899466 Change-Id: I309d7cf5918e8c17a84c3adfec4a8d7542336554 Reviewed-on: https://chromium-review.googlesource.com/c/1306997 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Original-Commit-Position: refs/heads/master@{#606543}(cherry picked from commit c20223cc6a70cae053ccf5673d8c10913d926453) Reviewed-on: https://chromium-review.googlesource.com/c/1329929 Reviewed-by: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/branch-heads/3578@{#620} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by fbeaufort@chromium.org
, Oct 29174 KB
174 KB View Download