[webkit-reviews] review denied: [Bug 50572] [GTK] Support the Mozilla-style Fullscreen Javascript API : [Attachment 75731] proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 50572: [GTK] Support the Mozilla-style Fullscreen Javascript API
https://bugs.webkit.org/show_bug.cgi?id=50572

Attachment 75731: proposed patch
https://bugs.webkit.org/attachment.cgi?id=75731&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75731&action=review

Looks good. The only two issues I have are preventing the documentation from
being in our public documentation and the naming of the configure variables.  

> WebKit/gtk/webkit/webkitwebsettings.cpp:913
> +    /**
> +    * WebKitWebSettings:enable-javascript-fullscreen:
> +    *
> +    * Enable or disable support for the Javascript Fullscreen API.
> +    *
> +    * Since: 1.3.8
> +    */

Let's leave this undocumented or unparsed now for now, so that it is private. I
don't think we should expose this until other Mac does.

> configure.ac:475
> +		 AC_HELP_STRING([--enable-javascript-fullscreen],

I think this should be --enable-fullscreen-api to match the flags on
build-webkit and the #ifdef.

> configure.ac:478
> +AC_MSG_RESULT([$enable_javascript_fullscreen])

Ditto.

> configure.ac:960
> +AM_CONDITIONAL([ENABLE_JAVASCRIPT_FULLSCREEN],[test
"$enable_javascript_fullscreen" = "yes"])

Ditto.

> configure.ac:1031
> + Javascript Fullscreen support			     :
$enable_javascript_fullscreen

Ditto.