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

commit daa2e3c8f7425c31c21d95f6632310290478acfd
Author: Tommy Steimel <steimel@chromium.org>
Date: Wed Jul 18 22:28:54 2018

[Media Controls] Only keep mouse events in node directly on play button

This CL makes further updates to the MediaControlOverlayPlayButton's
KeepEventInNode calculation to only keep mouse events in node if they
are on the internal play button itself. This fixes an issue on YouTube
Creator's custom blurring tool which prevents the user from creating
any blurs.

Bug:  864739 
Change-Id: I5106ba061c4bcce3594b84e0615ea45352db0f04
Reviewed-on: https://chromium-review.googlesource.com/1141097
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576233}
[modify] https://crrev.com/daa2e3c8f7425c31c21d95f6632310290478acfd/third_party/WebKit/LayoutTests/media/controls/click-on-side-of-overlay-play-button-is-propagated.html
[modify] https://crrev.com/daa2e3c8f7425c31c21d95f6632310290478acfd/third_party/blink/renderer/modules/media_controls/elements/media_control_overlay_play_button_element.cc
[modify] https://crrev.com/daa2e3c8f7425c31c21d95f6632310290478acfd/third_party/blink/renderer/modules/media_controls/elements/media_control_overlay_play_button_element.h

Fixed the issue. I know this is really late into the M68 cycle, but without this change the YT creator blur tool will be completely unusable in M68, so I'm requesting a merge to M68 here. It's a small and tested change

Thanks!

This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Thanks for the fix. How long has this been broken for and why are we hearing about this so late in the cycle? We'll need to request a postmortem for this, since it's a functional issue and discovered less than a week before stable. 

Sure thing. The short answer is it's been semi-broken since M67, but completely broken since July 10th. Long answer follows:

The YT creator blur tool has been semi-broken (minor-user-annoyance-level broken) since M67 (but we hadn't heard about it). However, in early July we received an issue from a Bulletin user of a video being unresponsive. The reason was that we weren't aggressive enough with KeepEventInNode for the overlay play button (we had intro'd a new overlay play button in M67 and had had to reduce the events kept in node to allow some websites to still receive events they had been dependent on), so I landed a fix in early July to make the overlay play button keep more events in node (but still allowing unused "click" events through since that's what we had seen issues with). <- My mistake was here when I should have been more thorough on fixing the whole underlying issue of KeepEventInNode not caring about where on the overlay play button the event is coming from (the button takes up the whole video above the timeline but only taps on the visible button itself actually cause a play/pause event). Therefore it ends up keeping events in node that it won't need to care about. Instead I just fixed it in a way that fixed that issue without re-breaking the KeepEventInNode issues we saw in M67. My reasoning was that it was already late in M68 so I wanted to keep the change as simple as possible.

However, then on Tuesday (July 17th) I got an email from the YT creator team about the minor-user-annoyance-level issue on M67. At that point I asked them to try canary since I knew the KeepEventInNode changes might solve their issues. Turns out the way it was working was completely breaking them because they depended on events outside the visible play button that we were now keeping. This fix does what we should have done before which was to only care about the events on the visible button itself (since those are the events that will cause the play/pause).



Side note: it looks like there was a canary released this morning but it doesn't look like this CL is on it according to the omahaproxy tool. Do you know when this will be in canary for testing?

Ah ok thanks for the quick fix and following up. Great to catch this before M68 hits stable. 

Seems like this missed yesterday's canary. Last changes that were in yesterday's canary are from 22hours ago, and this landed about 20 hours ago. This should be in tomorrow's canary. Can you please test this thoroughly, and if it looks good, we can merge this to M68 tomorrow. In addition, let's let it bake over the weekend and confirm again on Monday if there aren't any other regressions. 

How confident are you about this fix, in terms of how safe it is, chances of introducing new regressions?

Hey sorry to go back and forth on this but I spent more time thinking this through and it's probably for the best to wait for M69 for this. Instead, we should revert the CL from early July to go back to M67 funcitonality. There are known issues in that implementation, but it's been stable for weeks and there are no known show-stopping issues (whereas we know there are currently issues and can't be 100% that this fix will fix all of those). I discussed with the YT creator team and they can workaround for now. WDYT?

That CL merged into M68 branch here: crrev.com/c/1136751

If that's the safer choice, then I agree with the revert. 

Reverted. Changing this to M-69. Thanks!

Approving merge for the revert, as discussed in #9.

steimel@ has reverted this from branch 3440.