New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 719050 link

Starred by 1 user
Status: Fixed
Owner:
mlamouri@chromium.org
Closed: Sep 2017
Cc:
█ talo@chromium.org
mlamouri@chromium.org
█ dah...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Overflow menu is scoped to the media size, breaking it for small sizes

Project Member Reported by rachelis@chromium.org, May 5 2017 
Jon shared this bug today.

We allow the user to tap into the overflow menu, but the overflow menu itself is scoped to the size of the media player. 

This makes it impossible for the user to take any action in small instances of the media player.

Ccing mounir, tal, and jon. Is this just a bug, or did we choose this behavior (and if so, why?) :)
Screenshot_20170505-093806.png
341 KB View Download
Screenshot_20170505-093812.png
328 KB View Download

Comment 1 by mlamouri@chromium.org, May 11 2017

Components: Blink>Media>Controls
Labels: -OS-Android -Pri-2 -media-ux OS-All Pri-3
Status: Available (was: Untriaged)
This is specific to the web page. As you can see in this test page, a small video element doesn't block the overflow menu: https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html?width=100

However, the page has a wrapper around the element and sets "overflow: hidden;" which prevents the video to have any UI rendered outside of its container. Because we use basic HTML/CSS to show the overflow menu, we are limited by the CSS rules of the pages. Solutions to work around this might not be trivial.

Comment 3 by mlamouri@chromium.org, Jun 23 2017 

 bug 652952  is about making sure the overflow menu isn't rendered off-window while this bug is about the overflow menu being clipped outside of the iframes. We can solve  bug 652952  by being smart and show the menu on the other side if needed. I don't think it's worth the effort but we could. It wouldn't fix this bug though.

Comment 4 by mlamouri@chromium.org, Jul 26 2017

Owner: mlamouri@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 20 2017 

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

commit 534d590f44718950e08b37380eb24ae4366369ee
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Wed Sep 20 22:55:08 2017

Media Controls: show the overflow menu above all content.

This is preventing the overflow menu from being clipped by one of
the media element ancestor using `overflow: hidden;`.

Bug:  719050 
Change-Id: Iabe6a0716b524e5ca3631a81d496b305fa8c1f98
Reviewed-on: https://chromium-review.googlesource.com/585009
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503276}
[add] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/LayoutTests/media/controls/captions-menu-always-visible-expected.html
[add] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/LayoutTests/media/controls/captions-menu-always-visible.html
[add] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/LayoutTests/media/controls/overflow-menu-always-visible-expected.html
[add] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/LayoutTests/media/controls/overflow-menu-always-visible.html
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/LayoutTests/media/media-controls.js
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/Source/core/frame/DOMVisualViewport.h
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp
[modify] https://crrev.com/534d590f44718950e08b37380eb24ae4366369ee/third_party/WebKit/Source/modules/media_controls/resources/mediaControls.css

Comment 6 by mlamouri@chromium.org, Sep 20 2017

Status: Fixed (was: Started)

Comment 7 by mlamouri@chromium.org, Sep 20 2017

Labels: M-63
►

Sign in to add a comment