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

Issue 663680 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression:Media controller appears to be chopped after clicking on fullscreen icon of video.

Reported by vku...@etouch.net, Nov 9 2016

Issue description

Chrome Version:56.0.2913.0 (Official Build) 75d01e1f338c8a452f7d9aa80c2bfa463c0ce4f0-refs/heads/master@{#430459}-32/64 bit 
OS:Windows (7,8,8.1,10)

What steps will reproduce the problem?
(1)Launch chrome and navigate to http://dailymotion.github.io/hls.js/demo/
(2)After loading the page click on 'fullscreen' icon of video and observe the media controller at bottom.

Actual: Media controller appears to be chopped after clicking on fullscreen icon of video.

Expected: Media controller should be properly visible after clicking on fullscreen icon of video.

This is a regression issue broken in 'M56' and will soon update other info




 

Comment 1 by vku...@etouch.net, Nov 9 2016

Labels: OS-Linux
Manual regression range:
Good build: 56.0.2907.0 
Bad build: 56.0.2908.0 

Note: Issue not seen on Mac OS.
Actual_Video.mp4
548 KB View Download
Expected_Video.mp4
794 KB View Download
Labels: hasbisect-per-revision ReleaseBlock-Stable
Owner: liyuqian@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 56.0.2907.0 (Revision: 429169).
Bad build: 56.0.2908.0 (Revision: 429486).

You are probably looking for a change made after 429288 (known good), but no later than 429289 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/3347bb8ad251e778ed35c433c0246e0659f2402d..594ac7a7a33f562f6a3f419084b4b24655aa6d9f

Adding RB Label as this is a recent Regression. Please remove if not required.

@liyuqian -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.
Thank You.
Hmm... the CL itself doesn't seem suspicious to me as it's an unused flag (that we intend to use in the future). However, we recently had a big change that involved thousands of rebaselining so we have to land it manually (https://codereview.chromium.org/2457393004/). However, we reverted it earlier this week so it shouldn't affect anything now. Did you see this problem in the most recent build?

BTW, in my local chrome build, I got "your Browser does not support MediaSourceExtension / MP4 mediasource" in "http://dailymotion.github.io/hls.js/demo/". I tried add args "proprietary_codecs=1 ffmpeg_branding=Chrome" to gn but failed:

ERROR at build arg file (use "gn args <out_dir>" to edit):6:17: Undefined identifier
ffmpeg_branding=Chrome

Cc: liyuqian@chromium.org
Owner: msrchandra@chromium.org
After setting proprietary_codecs=true ffmpeg_branding="ChromeOS", I'm able to play the MP4. However, I couldn't reproduce the bug in Linux (my Goobuntu desktop) either with my change... Can you please verify that the bug still exists in the newer version of Chrome? (My Chrome source code is pulled yesterday.)
Still able to reproduce the issue on windows 7 using chrome version 56.0.2918.0.Please find the attached screen shot for the same.

liyuqian@ could you please look into this issue.

Comment 6 by liyuqian@google.com, Nov 15 2016

kavvaru@ : if this issue persists, I'm quite confident that it's not related with my change as it's reverted more than 1 week ago. Maybe run another bisect to identify the change that makes this problem?
Labels: -OS-Linux -OS-Windows OS-All
Owner: mlamouri@chromium.org
Taking this.
 mlamouri@, Any update on this please.
Cc: rbasuvula@chromium.org
Still able to reproduce the issue on windows 10.0 and Ubuntu 14.04 using chrome version 57.0.2931.0.

mlamouri@ could you please look into this issue.

Thank You!
Status: Started (was: Assigned)
I was away last week. Looking at this.
Cc: e...@chromium.org mlamouri@chromium.org foolip@chromium.org
Components: Blink>Fullscreen
Owner: foolip@chromium.org
The issue is that when going fullscreen, the code path that is meant to prevent controls from being cut is enabled because horizontal scrolling is not allowed. That should be fine but for some reason, during the fullscreen transition, there is a step where the absolute X offset is greater than 0. Again, that should be fine if it was followed with a layout with all the values set properly but it seems that it does not happen most of the time.

A quick and dirty fix would be to opt out when fullscreen because obviously, this feature doesn't make sense in fullscreen but it seems to be the result of a deeper issue that maybe should be looked at?

Assigning to foolip@ for him to have a look.
+eae@ for layout expertise.
Owner: mlamouri@chromium.org
#11, what is "the code path that is meant to prevent controls from being cut", do you mean MediaControls::computeWhichControlsFit?

Is the problem that the video's width is wrong at some point during the transition? Does it become correct again after the transition? If not I guess that's the issue, that the final size isn't reported?

If this is indeed a regression, bisecting down to the commit that really caused it would be great. Assigning back :)
Owner: foolip@chromium.org
Sorry for the bouncing :)

You are right that it is related to computePanelWidth(). The video size is properly reported but as you can see it is using the "std::min" of the video size and `visibleWidth - absoluteXOffset`. `visibleWidth` is also correct but `absoluteXOffset` seems to vary during the fullscreen animation and sometimes there is no layout after the animation so the latest computed width assumes that there is an offset so cuts the controls.
I wouldn't be surprised if this is fixed by  issue 627790 , and I'm spending all the time I can working towards that. Some interim workaround might be necessary though.

Is this in fact a regression? If it is due to funny things with fullscreen layout, I'd expect it to be as old as MediaControls::computeWhichControlsFit.
Oh sorry, it's not ::computeWhichControlsFit but LayoutMedia::computePanelWidth which is new. I guess if the fix is complex, I can land a workaround in there. What would you recommend to do?
Just to update, still able to reproduce the issue on windows 7 using chrome version 57.0.2943.0.
NextAction: 2017-01-02
I don't quite know what a workaround would look like without debugging myself. To clarify, is the problem in LayoutMedia::computePanelWidth transient during the fullscreen transition, so that resetting the controls later would fix the problem, or is absoluteXOffset wrong as long as in fullscreen.

If it's not transient, then I would guess it has something do to with the hacks surrounding LayoutFullScreen, as that replaces the layout object of the fullscreen element and has many known issues.

 Issue 240576  is not far away now, I'll set NextAction to January to see if that fixed it.
The issue is transient: absoluteXOffset is wrong when going fullscreen and it will sometimes be updated to the right value during the transition and sometimes it will not.

I will look into a quick fix to merge in the Beta channel.
NextAction: ----
Owner: mlamouri@chromium.org
Great, thanks!
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 7 2016

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

commit e4110ecc69aca6f4fb7b12ed9118844e8d883014
Author: mlamouri <mlamouri@chromium.org>
Date: Wed Dec 07 13:53:30 2016

Media Controls: don't try to cut the controls when the element is fullscreen.

During the fullscreen animation, the reported position of the media
element will be greater than 0 and it is sometimes not reset to 0 as it
should be, leaving the controls cut on the right side.

This a work around a fullscreen issue in order to avoid a regression.

BUG= 663680 
R=foolip@chromium.org

Review-Url: https://codereview.chromium.org/2550423003
Cr-Commit-Position: refs/heads/master@{#436937}

[modify] https://crrev.com/e4110ecc69aca6f4fb7b12ed9118844e8d883014/third_party/WebKit/Source/core/layout/LayoutMedia.cpp

Labels: Merge-Request-56

Comment 22 by dimu@chromium.org, Dec 8 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Thanks for the fix, we will verify in latest canary. If all looks good please merge to M56.
Labels: TE-Verified-M57 TE-Verified-57.0.2946.0
With response to comment #23:
Above issue is fixed on Latest Chrome Version:57.0.2946.0 (Official Build) 38c3eb61c737a8d3313ca8cd31b0c514c9d35b05-refs/heads/master@{#437422} on Mac (10.10.5)(10.11.5), Windows (7,8,8.1,10), Linux(14.04 LTS)

Project Member

Comment 25 by bugdroid1@chromium.org, Dec 9 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e768d9ee7b5adaa6b95c0869b931e6468685fa88

commit e768d9ee7b5adaa6b95c0869b931e6468685fa88
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Fri Dec 09 11:58:53 2016

Media Controls: don't try to cut the controls when the element is fullscreen.

During the fullscreen animation, the reported position of the media
element will be greater than 0 and it is sometimes not reset to 0 as it
should be, leaving the controls cut on the right side.

This a work around a fullscreen issue in order to avoid a regression.

BUG= 663680 
R=foolip@chromium.org

Review-Url: https://codereview.chromium.org/2550423003
Cr-Commit-Position: refs/heads/master@{#436937}
(cherry picked from commit e4110ecc69aca6f4fb7b12ed9118844e8d883014)

Review-Url: https://codereview.chromium.org/2561323002 .
Cr-Commit-Position: refs/branch-heads/2924@{#429}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/e768d9ee7b5adaa6b95c0869b931e6468685fa88/third_party/WebKit/Source/core/layout/LayoutMedia.cpp

Status: Fixed (was: Started)
Labels: TE-Verified-56.0.2924.28 TE-Verified-M56
Tested the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.12.1 using chrome version 56.0.2924.28.Able to see the media controller in full screen.

Adding TE-Verified labels.

Thanks,

Sign in to add a comment