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

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-02-18 09:39:22 PST ---
(From update of attachment 188901)
View in context: https://bugs.webkit.org/attachment.cgi?id=188901&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2940
> + * @visibility_state: a #WebKitWebViewPageVisibilityState

This says WebKitWebViewPageVisibilityState but the enum is called WebKitPageVisibilityState. I would avoid using Page here in any case, because it might be confused with WebKitWebPage API that is only available in the web extensions API. Maybe WebKitWebViewVisibiltyState, since our WebView is the equivalent to the WKPage in the C API.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2951
> + * see http://www.w3.org/TR/2011/WD-page-visibility-20110602/.

After reading this it's still not clear to me why and when a user of the API would call this method. Or is it only useful for testing? In such case it should be added to the C API which is what the tests use.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2960
> +    case WEBKIT_PAGE_VISIBILITY_STATE_VISIBLE:
> +        break;

I think it would be a bit clearer if state is set here instead of relying on the default value.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2965
> +        g_return_if_reached();

This should probably be g_assert_not_reached() instead, or even ASSERT_NOT_REACHED().

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