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

Issue 745944 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Deprecation message for HTMLVideoElement.webkitEnterFullscreen is misleading

Project Member Reported by crouleau@chromium.org, Jul 18 2017

Issue description

The deprecation message for video.webkitEnterFullscreen() says to use video.requestFullscreen(), but video.requestFullscreen() does not exist. Instead, it should say to use video.webkitRequestFullscreen(), which does exist.

Chrome version: 61.0.3159.5 (Official Build) dev (64-bit)

Repro Steps: 

1. Navigate to http://storage.googleapis.com/crouleau-shared/vids/roadtrip3.html (or any webpage that contains a video.
2. Open dev console.
3. Type in the following code:
"
video = document.getElementsByTagName('video')[0]
video.webkitEnterFullscreen()
"
4. See the output:
VM294:1 [Deprecation] 'HTMLVideoElement.webkitEnterFullscreen()' is deprecated. Please use 'Element.requestFullscreen()' instead.
(anonymous) @ VM294:1
5. Try the following code:
video.requestFullscreen()
6. See the following output:
VM391:1 Uncaught TypeError: video.requestFullscreen is not a function
    at <anonymous>:1:7
(anonymous) @ VM391:1
7. Try the following code:
video.webkitRequestFullscreen()
 
Cc: foolip@chromium.org
Components: -Internals>Media Blink>Media>Video
Summary: Deprecation message for HTMLVideoElement.webkitEnterFullscreen is misleading (was: Deprecation message is misleading)
The message is correct, but something is wrong if the new method is unvailable...
In third_party/WebKit/Source/core/dom/ElementFullscreen.idl

requestFullscreen() defined as below,
[RuntimeEnabled=FullscreenUnprefixed] void requestFullscreen();

And in /third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5,
"FullscreenUnprefixed" feature status is "test". requestFullscreen() working fine if i change the "FullscreenUnprefixed" feature statue to "stable".



Comment 4 by crouleau@google.com, Jul 21 2017

Just double-checked that this repros in Canary: 61.0.3163.0 (Official Build) canary (64-bit) (cohort: 64-Bit)

If the FullscreenUnprefixed feature is renaming that function, then it should also rename the function in the deprecation message.
Owner: crouleau@chromium.org
Status: Assigned (was: Available)
This seems like a nice low-priority bug to get my feet wet in the product code :). foolip@ is OOO for a few more days, but I'm happy to write some code and wait for his review.
FYI this is related to  issue 383813 .

https://chromium-review.googlesource.com/c/581948/ is out to fix it.
foolip@ commented on my changelist: 

"
There's a whole block of deprecation messages that I think ought to be treated similarly. The trouble is that the real advice for this is complicated, requiring the conditional use of all of requestFullscreen, mozRequestFullScreen, msRequestFullscreen and webkitRequestFullscreen.

The current deprecation messages aren't good, but suggesting to use only the prefixed APIs would also not be good. Would you be willing to write up some guidance in the style of https://goo.gl/rStTGz that could be linked?
"

I'm happy to start writing that document. I'll fill it in for requestFullscreen, and you can fill it in for the other functions. It seems like https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen is a good starting point.
Cc: fbeaufort@chromium.org jmedley@chromium.org crouleau@chromium.org
Owner: ----
Status: Available (was: Assigned)
Hmm, now that I think about this more, I don't think I can take this on. Maybe +Joe or +François can take this on.

Would it be okay to simply link to the MDN article: https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen ? Mozilla's doing a good job, and we can just help make that better rather than starting from scratch.

Comment 9 by jmedley@google.com, Aug 3 2017

@crouleau I'm as equally willing to do it as to accept your help. You say there are other message. I need to understand the scope. Look for a meeting invite in your inbox. 
foolip@ is a better bet to help you understand the scope. He is the one who knows what other messages there are. I was quoting him in comment #7. 
Components: Blink>Fullscreen
I don't think MDN succinctly answers the question "I'm using webkitEnterFullscreen, what should I do instead?"

I am working towards unprefixing, but even afterwards the message won't be very good, since it'll suggest using something that won't work everywhere for years to come. So, I do think new documentation is needed.
Project Member

Comment 12 by sheriffbot@chromium.org, Aug 3

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: -Blink>Fullscreen
Owner: mlamouri@chromium.org
Status: Assigned (was: Untriaged)
Looking at current data, it looks like the enter fullscreen methods are very rarely used but the exit ones are much more common. I've uploaded a CL that associated UKM to the use counters so it will help figure out where the usage is.

foolip@, I would be tempted to remove the webkitEnterFullscreen() and webkitEnterFullScreen() methods from HTMLVideoElement even if possible even if that means keeping the exit methods. Does this sound reasonable to you?
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 6

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

commit a5eb45f1e04bdd3cedf429f32ae1777a95646b07
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Mon Aug 06 21:21:30 2018

HTMLVideoElement: add UKM for usage of prefixed fullscreen video methods.

webkitEnterFullscreen() and webkitEnterFullScreen() are rarely used and
could be removed. The exit counter parts are significantly more common.
The UKM may help us identify the top sites using these methods and drive
their usage down.

Bug: 745944
Change-Id: Iec937c4f851049355b49bb59bb4d4e32ab8445f9
Reviewed-on: https://chromium-review.googlesource.com/1163835
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580990}
[modify] https://crrev.com/a5eb45f1e04bdd3cedf429f32ae1777a95646b07/chrome/browser/page_load_metrics/observers/use_counter/ukm_features.cc

Looks like these are the use counters:
https://www.chromestatus.com/metrics/feature/timeline/popularity/166
https://www.chromestatus.com/metrics/feature/timeline/popularity/167
https://www.chromestatus.com/metrics/feature/timeline/popularity/168
https://www.chromestatus.com/metrics/feature/timeline/popularity/169
https://www.chromestatus.com/metrics/feature/timeline/popularity/170
https://www.chromestatus.com/metrics/feature/timeline/popularity/171

The asymmetry is quite surprising, perhaps the exit variants are being called a lot when not actually in fullscreen. Or people are mixing element.webkitRequestFullscreen() + element.webkitExitFullscreen(), which wouldn't look odd.

Removing these APIs is tracked by issue 346236. A partial removal might work out, but there is a risk that one half of the API is used to feature detect usage of the whole, which leads to odd breakage. https://groups.google.com/a/chromium.org/d/msg/blink-dev/pIbpN_8Lqpg/tp9f71GsslIJ was such a case.

That being said, if we understand the usage via UKM and possibly httparchive research, then I wouldn't rule it out, as a step towards full removal.

Sign in to add a comment