Bug 125993 - [GTK] The fullscreen API should be enabled by default
Summary: [GTK] The fullscreen API should be enabled by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-19 06:00 PST by Enrique Ocaña
Modified: 2013-12-20 06:55 PST (History)
4 users (show)

See Also:

Attachments
Patch (4.00 KB, patch)
2013-12-19 06:11 PST, Enrique Ocaña 		no flags Details | Formatted Diff | Diff
Patch (4.00 KB, patch)
2013-12-20 01:37 PST, Enrique Ocaña 		no flags Details | Formatted Diff | Diff
Patch (4.63 KB, patch)
2013-12-20 05:14 PST, Enrique Ocaña 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Enrique Ocaña 2013-12-19 06:00:06 PST 
The enable-fullscreen setting should be enabled by default in WebKitGTK+.
Comment 1 Enrique Ocaña 2013-12-19 06:11:38 PST 
Created attachment 219644 [details]
Patch
Comment 2 WebKit Commit Bot 2013-12-19 06:14:17 PST 
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Enrique Ocaña 2013-12-20 01:37:22 PST 
Created attachment 219741 [details]
Patch
Comment 4 Enrique Ocaña 2013-12-20 01:38:10 PST 
Rebased and corrected the changelog.
Comment 5 Gustavo Noronha (kov) 2013-12-20 03:35:36 PST 
Comment on attachment 219741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219741&action=review

It sounds good to me, have this been discussed with Carlos, Phillippe or Martin? =)

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:834
> +            _("Whether the Mozilla style API should be enabled."),

Mozilla style? This should say the same thing as the other one, and have a Since of 2.4 I believe. Would be good to have the gtk-doc documentation header added here, I think.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:907
> +            _("Whether to enable the Javascriipt Fullscreen API"),

Javascript has 2 i's here, let's fix that too =)
Comment 6 Philippe Normand 2013-12-20 04:44:00 PST 
Comment on attachment 219741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219741&action=review

I agree with enabling this setting by default, fwiw

>> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:834
>> +            _("Whether the Mozilla style API should be enabled."),
> 
> Mozilla style? This should say the same thing as the other one, and have a Since of 2.4 I believe. Would be good to have the gtk-doc documentation header added here, I think.

It's actually an on-going effort at W3C nowadays. http://www.w3.org/TR/fullscreen/
Can you please update the strings accordingly?
Comment 7 Enrique Ocaña 2013-12-20 05:14:07 PST 
Created attachment 219752 [details]
Patch
Comment 8 Gustavo Noronha (kov) 2013-12-20 06:26:54 PST 
Comment on attachment 219752 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219752&action=review

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:836
> +    * Whether to enable the Javascript Fullscreen API. The API
> +    * allows any HTML element to request fullscreen display. See also
> +    * the current draft of the spec:
> +    * http://www.w3.org/TR/fullscreen/

Nit: these lines could be longer.
Comment 9 WebKit Commit Bot 2013-12-20 06:55:21 PST 
Comment on attachment 219752 [details]
Patch

Clearing flags on attachment: 219752

Committed r160909: <http://trac.webkit.org/changeset/160909>
Comment 10 WebKit Commit Bot 2013-12-20 06:55:23 PST 
All reviewed patches have been landed.  Closing bug.