| Summary: | [GTK] The fullscreen API should be enabled by default | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Enrique Ocaña <eocanha> | ||||||||
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | cgarcia, commit-queue, gustavo, mrobinson | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Enrique Ocaña
2013-12-19 06:00:06 PST
Created attachment 219644 [details]
Patch
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 Created attachment 219741 [details]
Patch
Rebased and corrected the changelog. 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 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? Created attachment 219752 [details]
Patch
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 on attachment 219752 [details] Patch Clearing flags on attachment: 219752 Committed r160909: <http://trac.webkit.org/changeset/160909> All reviewed patches have been landed. Closing bug. |