New issue
Advanced search Search tips

Issue 677900 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression

Blocking:
issue 679779



Sign in to add a comment

video tag with empty poster property doesn't unmute multiple times

Reported by kaufma...@gmail.com, Jan 3 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce the problem:
1. create a video tag with empty poster: <video controls="" poster="" style="width: 100%; height: 100%;">
2. play the video
3. use JavaScript video API to mute and unmute the video multiple times.
4. notice that you can't unmute the video and the volume is set to zero (although not requested - that is probably the problem)

What is the expected behavior?
video continues to mute and unmute

What went wrong?
1. cant unmute the video after few API mute/unmute attempts.
2. volume was set to zero although not requested.

Did this work before? Yes 

Chrome version: 55.0.2883.87  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 24.0 r0
 
Doesn't relate to the poster. Relates to the video playing wiht muted=true property.
Components: -Blink Blink>Media>Video

Comment 3 by ajha@chromium.org, Jan 4 2017

Labels: Needs-Bisect Needs-Triage-M55
Labels: Needs-Feedback
While tried with below html file:
<html>
<body>
<video controls="" poster="" style="width: 100%; height: 100%;">
</body>
</html>

Was unable to find the Video controls on Chrome to mute/unmute it.Though was seen on FireFox and was able to Mute and unmute it several times.
Could you please refer the screen cast of Win 10 using 55.0.2883.87, attached and let us know if any.
Please help providing sample test url or file to triage further.
677900_Jan_4.mp4
1.1 MB View Download
try the following:
http://codepen.io/Gordonos/pen/apbQob

HTML:
<video id="vid1" src="http://techslides.com/demos/sample-videos/small.mp4" controls>
</video>  
<button onClick="mute()">mute</button>
<button onClick="unmute()">unmute</button>

JS:
var vid = document.getElementById("vid1");
mute();

function mute(){
  vid.muted = true;
}

function unmute(){
  vid.muted = false;
}

1.click on the mute button (the button tag, not the native controls).
2. click play.
3. try to unmute. It doesn't work.
Labels: -Needs-Feedback -Needs-Bisect -Needs-Triage-M55 hasbisect-per-revision M-57 OS-Linux OS-Mac
Owner: mustaq@chromium.org
Status: Assigned (was: Unconfirmed)
Thanks for the update.
Able to reproduce the Issue on Win 10,Mac 10.12.2 and Ubuntu 14.04 using latest stable 5.0.2883.87/95 and canary 57.0.2970.0.

Note: Not very sure if the below bisect range is correct, considered bad when its reproduced once at least when tried for 3 times of reloading and playing it multiple times by mute and unmute for each page load.  

Bisect info:
Good: 55.0.2875.0
Bad : 55.0.2876.0

You are probably looking for a change made after 421810 (known good), but no later than 421811 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/bd027238e7f3d3c8096c1a62e9063d8cbf5d8a80..902a3d6303dfbfd49a24173777afcb4b70234a17

Review-Url: https://codereview.chromium.org/2375493005
mustaq@: Could you please take a look into this if its related to your change.
Status: Started (was: Assigned)
Here is an alternate way to repro, still not clear why this is happening:
- Play the video.
- Move the mouse along the gray control area to cross the right edge.
- Click unmute---it doesn't work.

Cc: chcunningham@chromium.org
More observations about #c7: in step 2, hovering on control area from the mute button towards the right edge caused the failure.

Even though this seems unrelated, disabling PointerEvents atually fixes the problem.

Chris: do you know if a <video> element relies on DOM events fired from Blink, or gets events directly from the browser?

Cc: liber...@chromium.org
The video tag's UI code is here:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp?rcl=0&l=286

I think this generates some HTML and probably relies on DOM events from blink (click). I'm not very familiar with the UI code. Here's the call stack I observe when someone clicks mute:

...
#6 0x7fdd69339a57 blink::MediaControlMuteButtonElement::defaultEventHandler()
#7 0x7fdd690a99e7 blink::EventDispatcher::dispatchEventPostProcess()
#8 0x7fdd690a8bf5 blink::EventDispatcher::dispatch()
#9 0x7fdd690c1829 blink::MouseEventDispatchMediator::dispatchEvent()
#10 0x7fdd690a8121 blink::EventDispatcher::dispatchEvent()
#11 0x7fdd6937ff08 blink::MouseEventManager::dispatchMouseEvent()
#12 0x7fdd6937507d blink::EventHandler::handleMouseReleaseEvent()
#13 0x7fdd710b3a72 blink::PageWidgetEventHandler::handleMouseUp()
#14 0x7fdd7114065f blink::WebViewImpl::handleMouseUp()
#15 0x7fdd710b3738 blink::PageWidgetDelegate::handleInputEvent()
#16 0x7fdd71142b28 blink::WebViewImpl::handleInputEvent()
#17 0x7fdd7534b753 content::RenderWidgetInputHandler::HandleInputEvent()
...
Thanks Chris, this is useful.

The video seems disabled whenever I run my local build of Chrome for debugging (in Linux). Any clue why this is happening?
I think you may just need to set proprietary_codecs = true in your gn config. 

I forked the pen from comment 5 and added a little interval to monitor volume and muted. What I find is that, when muted = true, simply mousing over the the volume slider (no clicking) will change the volume value to 0 and muted to false.

When muted = false, mousing over the slider has no effect. 

http://codepen.io/anon/pen/VPLzrX
Labels: PointerEvent Hotlist-Input-Dev
chcunningham@: does #c11 happen only when pointer events are enabled (through chrome://flags/#enable-pointer-events)? If yes, this is clearly the same problem. I suspect mouseover double-handling in UI code because mousing over now fires "pointerover" first then "mouseover".

Cc: mlamouri@chromium.org
+mlamouri

the mute button ignores anything that's not a click, though:  https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp?rcl=0&l=286

there is some special processing that might catch pointer/mouseover, but it's only used to eat events.  the mute button should ignore them.
HTMLMediaElement::setMuted(false) sets the volume to effectiveMediaVolume() which is zero for muted state. Still not clear why non-click events can trigger this. Any chance other callers of effectiveMediaVolume() are accidentally triggering this? E.g. updatePlayState()?


Cc: mustaq@chromium.org
Owner: ----
Status: Available (was: Started)
Making the bug available for a better owner, perhaps someone from Chrome video team.
Owner: chcunningham@chromium.org
Taking a look at the media side.

> chcunningham@: does #c11 happen only when pointer events are enabled

The issue goes away when pointer events are disabled. 

I should clarify that my demo in comment #11 is actually the exact same manifestation of the bug from comment #5. The key in both cases is mousing over the volume slider while the player is in the muted state. If you follow the repro steps from comment #5, but are careful to avoid the volume slider, the repro disappears. 

> Any chance other callers of effectiveMediaVolume() are accidentally triggering this?

effectiveMediaVolume() is correct to return 0 when muted. The key is that the m_volume member continues to hold its pre-muted value. For reasons unknown, mousing over the slider while muted=true is causing a call to HTMLMediaElement::setVolume(0), which clears out the pre-muted value in m_volume. 

Will post this strange mouse over call stack shortly.
Here is the code that causes the problem: MediaControlVolumeSliderElement::defaultEventHandler()

This should include pointer events now, or even better, should only handle pointer events.

I suspect a similar pointerover/mouseover bug for MediaControlTimelineElement::defaultEventHandler() but couldn't repro.

This is the stack when the volume is set to zero on mouse over.

#6 0x7f3f30279c08 blink::HTMLMediaElement::setVolume()
#7 0x7f3f303913a2 blink::MediaControlVolumeSliderElement::defaultEventHandler()
#8 0x7f3f300feb90 blink::EventDispatcher::dispatchEventPostProcess()
#9 0x7f3f300fdbf5 blink::EventDispatcher::dispatch()
#10 0x7f3f301184a3 blink::PointerEventDispatchMediator::dispatchEvent()
#11 0x7f3f300fd121 blink::EventDispatcher::dispatchEvent()
#12 0x7f3f303d83e7 blink::PointerEventManager::dispatchPointerEvent()
#13 0x7f3f303c5322 blink::BoundaryEventDispatcher::sendBoundaryEvents()
#14 0x7f3f303d8c7c blink::PointerEventManager::setNodeUnderPointer()
#15 0x7f3f303d88b4 blink::PointerEventManager::processCaptureAndPositionOfPointerEvent()
#16 0x7f3f303da1bf blink::PointerEventManager::sendMousePointerEvent()
#17 0x7f3f303c96fa blink::EventHandler::handleMouseMoveOrLeaveEvent()
#18 0x7f3f303c9640 blink::EventHandler::handleMouseMoveOrLeaveEvent()
#19 0x7f3f303c8f66 blink::EventHandler::handleMouseMoveEvent()
#20 0x7f3f3810889a blink::PageWidgetEventHandler::handleMouseMove()
#21 0x7f3f38108785 blink::PageWidgetDelegate::handleInputEvent()
#22 0x7f3f38197b28 blink::WebViewImpl::handleInputEvent()
#23 0x7f3f3c3a0753 content::RenderWidgetInputHandler::HandleInputEvent()

Blocking: 679779
Let me take a stab at this since this is the first case we discovered for PointerEvents. :(
Sent out crrev.com/2622273003 for review.
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 25 2017

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

commit 74fce5949bdf05a92c2bc0bd98e6e3e977c55376
Author: mustaq <mustaq@chromium.org>
Date: Wed Jan 25 16:25:58 2017

Fixed volume slider element event handling

MediaControlVolumeSliderElement::defaultEventHandler has making
redundant calls to setVolume() & setMuted() on mouse activity. E.g. if
a mouse click changed the slider position, the above calls were made 4
times, once for each of these events: mousedown, input, mouseup,
DOMActive, click. This crack got exposed when PointerEvents are enabled
by default on M55, adding pointermove, pointerdown & pointerup to the
list.

This CL fixes the code to trigger the calls to setVolume() & setMuted()
only when the slider position is changed. Also added pointer events to
certain lists of mouse events in the code.

BUG= 677900 

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

[modify] https://crrev.com/74fce5949bdf05a92c2bc0bd98e6e3e977c55376/third_party/WebKit/Source/core/html/shadow/MediaControlElementTypes.h
[modify] https://crrev.com/74fce5949bdf05a92c2bc0bd98e6e3e977c55376/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp

Status: Fixed (was: Available)
Labels: ReleaseBlock-Beta
Labels: -hasbisect-per-revision Merge-Request-57
Owner: mustaq@chromium.org
Status: Verified (was: Fixed)
Verified the fix in Win Canary (58.0.2993.0).
Project Member

Comment 26 by sheriffbot@chromium.org, Jan 26 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 27 by bugdroid1@chromium.org, Jan 26 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69bc0646f147fea740958753d085324cd6b5d2e4

commit 69bc0646f147fea740958753d085324cd6b5d2e4
Author: Mustaq Ahmed <mustaq@google.com>
Date: Thu Jan 26 16:53:59 2017

Fixed volume slider element event handling

MediaControlVolumeSliderElement::defaultEventHandler has making
redundant calls to setVolume() & setMuted() on mouse activity. E.g. if
a mouse click changed the slider position, the above calls were made 4
times, once for each of these events: mousedown, input, mouseup,
DOMActive, click. This crack got exposed when PointerEvents are enabled
by default on M55, adding pointermove, pointerdown & pointerup to the
list.

This CL fixes the code to trigger the calls to setVolume() & setMuted()
only when the slider position is changed. Also added pointer events to
certain lists of mouse events in the code.

BUG= 677900 

Review-Url: https://codereview.chromium.org/2622273003
Cr-Commit-Position: refs/heads/master@{#446032}
(cherry picked from commit 74fce5949bdf05a92c2bc0bd98e6e3e977c55376)

Review-Url: https://codereview.chromium.org/2654183005 .
Cr-Commit-Position: refs/branch-heads/2987@{#106}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/69bc0646f147fea740958753d085324cd6b5d2e4/third_party/WebKit/Source/core/html/shadow/MediaControlElementTypes.h
[modify] https://crrev.com/69bc0646f147fea740958753d085324cd6b5d2e4/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp

Sign in to add a comment