We currently have the following custom APIs for the <video> element in order to enter full-screen: readonly attribute boolean webkitSupportsFullscreen; readonly attribute boolean webkitDisplayingFullscreen; void webkitEnterFullScreen() raises (DOMException); void webkitExitFullScreen(); Note that the properties use a lower-case "s" for "screen" while the methods use an uppercase "S" for "Screen". I believe the "S" should be uppercase in all instances, but more importantly, the naming should be consistent.
This inconsistency extends to the rest of the code, so I'd like us to go through and make it consistent throughout when fixing this. When doing fullscreen for Windows, I was careful to use Fullscreen throughout. That seems to be the more common usage in the rest of the code (although not by much) and I think it makes sense if you consider "fullscreen" to be one word rather than two.
<rdar://problem/7729165>
Created attachment 50643 [details] Patch I polled the audience and "fullscreen" won.
Comment on attachment 50643 [details] Patch > Index: WebCore/html/HTMLVideoElement.idl > =================================================================== > --- WebCore/html/HTMLVideoElement.idl (revision 55928) > +++ WebCore/html/HTMLVideoElement.idl (working copy) > @@ -33,6 +33,10 @@ module html { > > readonly attribute boolean webkitSupportsFullscreen; > readonly attribute boolean webkitDisplayingFullscreen; > + > + [NeedsUserGestureCheck] void webkitEnterFullscreen() > + raises (DOMException); > + void webkitExitFullscreen(); > > [NeedsUserGestureCheck] void webkitEnterFullScreen() > raises (DOMException); I think you file a bugzilla to remove the old versions, and add a comment referencing it. > Index: WebKit/mac/ChangeLog > =================================================================== > --- WebKit/mac/ChangeLog (revision 55944) > +++ WebKit/mac/ChangeLog (working copy) > @@ -1,3 +1,31 @@ > +2010-03-12 Beth Dakin <bdakin@apple.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Fix for https://bugs.webkit.org/show_bug.cgi?id=34942 Fullscreen > + API naming is inconsistent > + -and corresponding- > + <rdar://problem/7729165> > + > + This patch changes all occurrences of "fullScreen" to the more > + popular "fullscreen." > + > + * Plugins/Hosted/NetscapePluginHostProxy.h: > + (WebKit::NetscapePluginHostProxy::isFullscreenWindowShowing): > + * Plugins/Hosted/NetscapePluginHostProxy.mm: > + (WebKit::NetscapePluginHostProxy::NetscapePluginHostProxy): > + (WebKit::NetscapePluginHostProxy::didEnterFullscreen): > + (WebKit::NetscapePluginHostProxy::didExitFullscreen): > + (WebKit::NetscapePluginHostProxy::setFullscreenWindowIsShowing): > + (WKPCSetFullscreenWindowIsShowing): > + * Plugins/Hosted/WebKitPluginClient.defs: > + * Plugins/WebNetscapePluginView.mm: > + (-[WebNetscapePluginView _workaroundSilverlightFullscreenBug:]): > + (-[WebNetscapePluginView _createPlugin]): > + (-[WebNetscapePluginView _destroyPlugin]): > + * WebView/WebVideoFullscreenHUDWindowController.mm: > + (-[WebVideoFullscreenHUDWindowController windowDidLoad]): I think it's fine if you don't change the plugin-related code in this patch; up to you. r=me
Fixed with r55946.
This may have broken video tests on Snow Leopard: media/video-source-none-supported.html expected actual diff pretty diff plugins/keyboard-events.html expected actual diff pretty diff http/tests/media/video-cancel-load.html expected actual diff pretty diff http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r55947%20(6631)/results.html But the Snow Leopard Bot it so flakey it's difficult to tell.