Deprecation message for HTMLVideoElement.webkitEnterFullscreen is misleading |
||||||||
Issue descriptionThe 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()
,
Jul 18 2017
The message is correct, but something is wrong if the new method is unvailable...
,
Jul 20 2017
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".
,
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.
,
Jul 21 2017
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.
,
Jul 21 2017
FYI this is related to issue 383813 . https://chromium-review.googlesource.com/c/581948/ is out to fix it.
,
Aug 1 2017
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.
,
Aug 2 2017
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.
,
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.
,
Aug 3 2017
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.
,
Aug 3 2017
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.
,
Aug 3
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
,
Aug 6
,
Aug 6
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?
,
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
,
Aug 7
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 |
||||||||
Comment 1 by dalecur...@chromium.org
, Jul 18 2017Components: -Internals>Media Blink>Media>Video