[Webkit-unassigned] [Bug 97324] [GTK] Add support for Page Visibility

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=97324


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #187854|review?                     |review-
               Flag|                            |




--- Comment #13 from Martin Robinson <mrobinson at webkit.org>  2013-02-13 09:37:39 PST ---
(From update of attachment 187854)
View in context: https://bugs.webkit.org/attachment.cgi?id=187854&action=review

Looks good, apart from a couple nits.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2947
> + * Override the page visibility in @webView. The page visibility state will be set to @visibility_state.
> + * Can be used to set the page visibility state before @web_view is mapped.
> + *
> + * When the operation is finished, Java Script visibility change event will be triggered unless @initial_state flag is set.
> + * For more information about Page Visibility see http://www.w3.org/TR/2011/WD-page-visibility-20110602/.

For the sake of readability, do you mind making these lines about as long as the other documentation? "Java Script" should be "JavaScript". :)

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:167
> + * @WEBKIT_PAGE_VISIBILITY_STATE_VISIBLE: The document contained by the page is at least partially visible at on at least one screen. This is the same condition under which the hidden attribute is set to false.
> + * @WEBKIT_PAGE_VISIBILITY_STATE_HIDDEN: The document contained by the page is not visible at all on any screen.
> + *
> + * Enum values to specify the different visibility states for a #WebKitWebView

Perhaps make this a bit terser? I'm thinking something like "The page is at least partially visible and the hidden attribute is set to false." and then "The page is not visible at all on the screen."

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:171
> +typedef enum
> +{

The curly brace here should be on the same line as the enum.

> Source/WebKit2/UIProcess/API/gtk/tests/WebViewTest.h:49
> +    void showInWindow(GtkWindowType = GTK_WINDOW_POPUP);

You aren't using the "mapped" state of the widget here, so why do you actually need to show the window?

> Tools/DumpRenderTree/gtk/TestRunnerGtk.cpp:966
> +    WebKitWebView* webView = webkit_web_frame_get_web_view(mainFrame);
> +    DumpRenderTreeSupportGtk::resetPageVisibility(webView);

Instead of exposing more API, why not just call  DumpRenderTreeSupportGtk::setPageVisibility(webView, WebCore::PageVisibilityStateVisible);

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.