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

Issue 703210 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Android MediaRouter only (left Chro...
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

MediaCustomControlsFullscreenDetector not listening to events properly

Project Member Reported by zqzh...@chromium.org, Mar 20 2017

Issue description

This happens when the <video> tag is written in the HTML file, i.e. not by adding the element to the document.
 
Labels: -Restrict-View-Google
Removing Restrict-View-Google as there is nothing special here.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 20 2017

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

commit d9d39e2308b617bad51c86f9703828a2fbef95d8
Author: zqzhang <zqzhang@chromium.org>
Date: Mon Mar 20 19:17:08 2017

Fix the conditions when fullscreen detector listeners are registered

Previously, we rely on the DOMNode[Inserted/Removed]IntoDocument events
to attach/detach the detector. However this won't work if the video
element is created from the parsing process. This CL fixes the issue by
overriding insertedInto() and removedFrom() callbacks, which always
works regardless of how the video element is inserted into the document.

BUG= 703210 

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

[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.cpp
[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.h
[modify] https://crrev.com/d9d39e2308b617bad51c86f9703828a2fbef95d8/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetectorTest.cpp

Labels: Merge-Request-58
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 21 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

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

Comment 5 by bugdroid1@chromium.org, Mar 21 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c

commit 8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c
Author: Zhiqiang Zhang <zqzhang@google.com>
Date: Tue Mar 21 21:15:39 2017

Fix the conditions when fullscreen detector listeners are registered

Previously, we rely on the DOMNode[Inserted/Removed]IntoDocument events
to attach/detach the detector. However this won't work if the video
element is created from the parsing process. This CL fixes the issue by
overriding insertedInto() and removedFrom() callbacks, which always
works regardless of how the video element is inserted into the document.

BUG= 703210 

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

Review-Url: https://codereview.chromium.org/2767763002 .
Cr-Commit-Position: refs/branch-heads/3029@{#343}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.cpp
[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.h
[modify] https://crrev.com/8ac66c6ce3966669bb2f3db5bd7a454a96ff4c0c/third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetectorTest.cpp

Status: Fixed (was: Started)

Comment 7 by dah...@chromium.org, Mar 29 2017

Blocking: -679364

Sign in to add a comment