Issue metadata
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 descriptionChrome 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
,
Nov 9 2016
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.
,
Nov 9 2016
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
,
Nov 11 2016
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.)
,
Nov 14 2016
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.
,
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?
,
Nov 16 2016
Taking this.
,
Nov 22 2016
mlamouri@, Any update on this please.
,
Nov 25 2016
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!
,
Nov 29 2016
I was away last week. Looking at this.
,
Nov 29 2016
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.
,
Dec 1 2016
#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 :)
,
Dec 1 2016
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.
,
Dec 1 2016
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.
,
Dec 1 2016
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?
,
Dec 6 2016
Just to update, still able to reproduce the issue on windows 7 using chrome version 57.0.2943.0.
,
Dec 6 2016
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.
,
Dec 6 2016
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.
,
Dec 6 2016
Great, thanks!
,
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
,
Dec 7 2016
,
Dec 8 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 8 2016
Thanks for the fix, we will verify in latest canary. If all looks good please merge to M56.
,
Dec 9 2016
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)
,
Dec 9 2016
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
,
Dec 9 2016
,
Dec 14 2016
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 |
|||||||||||||||||||||||
Comment 1 by vku...@etouch.net
, Nov 9 2016548 KB
548 KB View Download
794 KB
794 KB View Download