[Webkit-unassigned] [Bug 109422] [Qt] Add Page Visibility API support

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Pierre Rossi <pierre.rossi at gmail.com>  2013-02-19 05:48:04 PST ---
(From update of attachment 189060)
View in context: https://bugs.webkit.org/attachment.cgi?id=189060&action=review

Almost there I think. The only thing I'd like cleaned up still is the introduction of a QWebPage::PageVisibility enum, which should address most of my remaining comments, but I think it's mostly my fault for not making that super clear when I mentioned it before on IRC.

> Source/WebKit/qt/WebCoreSupport/QWebPageAdapter.cpp:1490
> +  \sa Page::setVisibilityState(PageVisibilityState visibilityState, bool isInitialState)

QWebPageAdapter and friends aren't public API, no documentation needed here.

> Source/WebKit/qt/WidgetApi/qwebpage.cpp:930
> +  \sa QWebPageAdapter::setVisibilityState(int visibilityState, bool isInitialState)

Not public API, so no need for a "See Also" entry here. But there could be mention that QWebView and QGraphicsWebView already do "the right thing"™ and that exposing this as public API in QWebPage is more geared towards "headless" use cases.

> Source/WebKit/qt/WidgetApi/qwebpage.cpp:932
> +void QWebPage::setVisibilityState(int visibilityState, bool isInitialState)

here, the integer parameter is disturbing because it's a public API and this is not a very Qt-ish thing to do. I think a QWebPage::PageVisibility enum needs to be introduced and documented.
For the sake of simplicity, it can follow the values of the corresponding WebCore enum and cast back in forth to and from int through the QtWebKit layer, but in that case it's probably best to add a comment before the enum in QWebPage mentioning it has to be kept in sync with the WebCore one. (not something that's likely to change much on either side I believe).

> Source/WebKit/qt/WidgetApi/qwebview.cpp:733
> +            d->page->setVisibilityState(WebCore::PageVisibilityStateVisible, false);

Small layering violation here: we can't use WebCore types or enums in libQtWebKitWidgets. But the QWebPage version can be used once it's introduced.

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