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

Issue 655642 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 1
Type: Bug

Blocking:
issue 638807



Sign in to add a comment

Media controls overflow menu shows cast option when disableRemotePlayback is set

Project Member Reported by mlamouri@chromium.org, Oct 13 2016

Issue description

Anton, can you have a look at this? It's probably a simple check that was removed by mistake or need to be added specifically for the overflow menu item.

Please reassign to me if you don't think you can have a look by EOW. I should have time later this week but at a conference today.
 
Blocking: 638807
Do you have a repro case?

I tried it with https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html.

I can see that if I add the disableRemotePlayback attribute programmatically in dev console, the media elements bar is not updated - even in the case all controls fit, the Cast button is not removed from the controls bar.

However, if I then resize the element, the Cast button is shown in neither controls bar not in the overflow list.

So the bug I'm seeing is that changing the attribute doesn't refresh the media controls.
Cc: avayvod@chromium.org
Labels: Needs-Feedback
Owner: tkonch...@chromium.org
Original report is https://bugs.chromium.org/p/chromium/issues/detail?id=638807#c35

+tkonchada@ can you confirm that what you saw is what avayvod@ pointed above?
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 15 2016

Cc: -avayvod@chromium.org
Labels: -Needs-Feedback Merge-Request-55
Owner: avayvod@chromium.org
Status: Started (was: Assigned)
The fix seems simple enough to be merged.

Comment 6 by dimu@chromium.org, Oct 16 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)

Comment 7 by gov...@chromium.org, Oct 17 2016

Please merge your change to M55 branch 2883 before 4:00 PM PT, Monday (10/17/16) in order to make to last M55 dev release before Beta promotion. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 17 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed

commit e6e6ba6154fa6ee935099f9bab414eb516a3e9ed
Author: Anton Vayvod <avayvod@google.com>
Date: Mon Oct 17 15:46:53 2016

[Cast] Refresh cast button visibility when disableRemotePlayback attribute changes

BUG= 655642 

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

Review URL: https://codereview.chromium.org/2429543002 .

Cr-Commit-Position: refs/branch-heads/2883@{#151}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp

Status: Fixed (was: Started)
Cc: tkonch...@chromium.org
Labels: Needs-Feedback
The issue mentioned in https://bugs.chromium.org/p/chromium/issues/detail?id=638807#c35  
is that 
1. navigate to https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
2. Check the option "Disable cast"
3. Right click on the video and observe the context menu options

Expected behavior : cast option should be disabled
Actual Behavior : cast option is enabled

Please find the screenshot

This issue is still seen on latest M55 55.0.2883.18 as well

avayvod@, Could you please let us know the manual TEST steps to verify the fix in comment #8
Screen Shot 2016-10-18 at 11.50.07 AM.png
669 KB View Download
Labels: -Needs-Feedback
Thanks for the report, I was not aware that we had a "Cast" option from the context menu.

I've filed  bug 656965  for this. It's not part of the feature being tested in bug 638807.
Re #c10 - add/remove attribute when the controls are shown - before the change the button wouldn't be hidden immediately, only after something triggers controls repainting (like hiding them and showing them again if the video was playing). Now it should be instant.

I'll look into  issue 656965  separately.
To reproduce the issue fixed here:
- connect the device to a network with Chromecast, open Chrome
- navigate to https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
- play the video and wait until the cast button appears in the controls
- pause the video so that controls don't auto-hide
- check the Disable cast box

ER: cast button disappears from the controls bar (and overflow menu list)
AR: cast button disappears only after the controls bar is caused to redraw (e.g. by resuming the video, etc)
Cc: pbomm...@chromium.org aska...@chromium.org
+Ashwini for verification
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed

commit e6e6ba6154fa6ee935099f9bab414eb516a3e9ed
Author: Anton Vayvod <avayvod@google.com>
Date: Mon Oct 17 15:46:53 2016

[Cast] Refresh cast button visibility when disableRemotePlayback attribute changes

BUG= 655642 

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

Review URL: https://codereview.chromium.org/2429543002 .

Cr-Commit-Position: refs/branch-heads/2883@{#151}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/e6e6ba6154fa6ee935099f9bab414eb516a3e9ed/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp

Comment 16 by dimu@google.com, Nov 4 2016

[Automated comment] removing mislabelled merge-merged-2840

Comment 17 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment