Summary: | Fullscreen API naming is inconsistent | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <ml> | ||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bdakin, cmarrin, eric.carlson, eric, simon.fraser | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.6 | ||||||
Attachments: |
|
Description
Antoine Quint
2010-02-15 07:26:51 PST
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. 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 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. |