Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(235)

Issue 210763004: Remove special case for HTMLMediaElement in determineDispatchBehavior (Closed)

Created:
6 years, 9 months ago by philipj_slow
Modified:
6 years, 9 months ago
Reviewers:
hayato
CC:
blink-reviews
Visibility:
Public.

Description

Remove special case for HTMLMediaElement in determineDispatchBehavior This was added in WebKit r87692 in order to fix "Scrubbing a Vimeo movie when in fullscreen stops playback; no way to make it start again": https://bugs.webkit.org/show_bug.cgi?id=61717 The commit message and bug comments makes it clear that this special case is no longer needed: 1. The media controls fullscreen button now uses largely the same code path as Element::webkitRequestFullscreen. "Video-only full screen is a mode where we use the shadow DOM as an implementation detail that should not be detectable by the web content" is no longer true at all, since Web content can observe the events fired, etc. 2. Scrubbing no longer uses HTMLMediaElement::setPausedInternal(), which looks like the cause of "no way to make it start again". BUG=none TEST=None originally added with the code, but existing tests pass. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170017

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -10 lines) Patch
M Source/core/events/EventPath.cpp View 2 chunks +0 lines, -10 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
philipj_slow
hayato, can you PTAL? I see that you've been working in this file a bit ...
6 years, 9 months ago (2014-03-25 09:10:14 UTC) #1
hayato
LGTM. Happy to see the special case was gone.
6 years, 9 months ago (2014-03-26 03:13:45 UTC) #2
philipj_slow
The CQ bit was checked by philipj@opera.com
6 years, 9 months ago (2014-03-26 04:44:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/philipj@opera.com/210763004/1
6 years, 9 months ago (2014-03-26 04:44:45 UTC) #4
commit-bot: I haz the power
6 years, 9 months ago (2014-03-26 05:07:12 UTC) #5
Message was sent while issue was closed.
Change committed as 170017

Powered by Google App Engine
This is Rietveld 408576698