Media Context Menu does not work for Loop/Controls/Picture-in-Picture with OOPIF |
||||||
Issue descriptionSTR: 1. Open https://mounirlamouri.github.io/sandbox/autoplay/unmuted-cross-origin-iframe.html 2. Attempt to toggle "Loop", "Show controls" or "Picture-in-Picture" Expected result: work as if the video was in the main frame. Actual result: does not work. Cast, copy and open actions seem to work fine. +apacible@ and fbeaufort@ as they changed some Picture-in-Picture context menu recently and this could be a good follow-up. I'm particularly worried that we may end up with the item not working on embeds.
,
Jun 12 2018
Issue 776807 is primarily about making sure the coordinates passed to the plugin are correct, but in this case the context menu commands fail because WebViewImpl doesn't know about the elements in the OOPIF subframe, and the check at https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/exported/web_view_impl.cc?rcl=8b6e2bfbde8c38653901c6dcfe5baa4e78c245f9&l=2946 fails and early outs. The appropriate fix here (I think) is to move PerformMediaPlayerAction to a location (WebFrameWidgetImpl?) where it can serve both main frames and OOPIFs. I can look into this, though maybe not in the next few days.
,
Jun 26 2018
I got annoyed enough by this while dogfooding the feature that I wrote a fix.
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fdff8bf75117f760384d5f8ec775fa3827335423 commit fdff8bf75117f760384d5f8ec775fa3827335423 Author: Mounir Lamouri <mlamouri@chromium.org> Date: Wed Jun 27 21:49:53 2018 OOPIF: enable context menu media actions on OOPIF. This will allow actions such as "loop", "show controls" or the new "Picture-in-Picture" to work on OOPIF. Bug: 851502 Change-Id: Ie2937e908866511c5dbdd0bbf31cf90b3704b859 Reviewed-on: https://chromium-review.googlesource.com/1115582 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/heads/master@{#570911} [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/chrome/browser/renderer_context_menu/render_view_context_menu.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/browser/renderer_host/render_view_host_impl.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/common/frame_messages.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/common/view_messages.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/public/browser/render_frame_host.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/public/browser/render_view_host.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/renderer/render_frame_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/renderer/render_frame_impl.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/renderer/render_view_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/content/renderer/render_view_impl.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/public/web/web_local_frame.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/public/web/web_view.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/renderer/core/exported/web_view_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/renderer/core/exported/web_view_impl.h [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/renderer/core/frame/web_local_frame_impl.cc [modify] https://crrev.com/fdff8bf75117f760384d5f8ec775fa3827335423/third_party/blink/renderer/core/frame/web_local_frame_impl.h
,
Jun 27 2018
,
Jun 28 2018
Able to reproduce the issue on Mac 10.13.3 using chrome build without fix on enabling the #site-per-process flag. Tested the fix on Mac 10.13.3, Win-10 and Ubuntu 17.10 using latest chrome version #69.0.3475.0 as per the comment #0. Attaching screen cast for reference. Observed that on toggling the "Show controls" from context menu worked as expected but toggling the "loop" option from context menu did nothing in the video. mlamouri@ - Could you please check the attached screen cast and please help us in confirming the fix. Thanks...!!
,
Jun 28 2018
Loop would only apply when the video reaches the end. The fact that loop is checked means that it worked. Thanks for verifying and for the recording! :) |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by fbeaufort@chromium.org
, Jun 12 2018