[webkit-reviews] review denied: [Bug 69127] [EFL] Enabling the Page Visibility API : [Attachment 109273] Patch

[bugzilla-daemon at webkit.org]

Raphael Kubo da Costa <kubo at profusion.mobi> has denied Dongwoo Joshua Im
<dw.im at samsung.com>'s request for review:
Bug 69127: [EFL] Enabling the Page Visibility API
https://bugs.webkit.org/show_bug.cgi?id=69127

Attachment 109273: Patch
https://bugs.webkit.org/attachment.cgi?id=109273&action=review

------- Additional Comments from Raphael Kubo da Costa <kubo at profusion.mobi>
View in context: https://bugs.webkit.org/attachment.cgi?id=109273&action=review


> ChangeLog:12
> +	   * Source/cmake/OptionsEfl.cmake:
> +	   * Source/cmakeconfig.h.cmake:

The description above is too generic; please describe what is being changed in
these specific files.

> Source/WebKit/efl/ChangeLog:12
> +	   Browser(or, other application) should call this newly added function

> +	   when page visibility is changed.

Do you mean users should call the API when the page visibility changes or when
they want to change it? If it is really the former, how are they informed that
this change has happened?

> Source/WebKit/efl/ChangeLog:15
> +	   (ewk_view_visibility_state_set): Setter function of EFL port.

Don't you also need a getter?

> Source/WebKit/efl/ewk/ewk_view.h:2183
> + * When the visibility status of page is changed,
> + * this API should be called by the application who use WebKit, such as
browser.

Weird line break here, could you make both lines have approximately the same
width?

> Source/WebKit/efl/ewk/ewk_view.h:2185
> + * When the visibility status of page is changed,
> + * this API should be called by the application who use WebKit, such as
browser.
> + * Then the web application will get the visibility status of the page on
which it runs,
> + * it could stop or slow down if the visibility of the page is hidden.

The whole description is a bit unclear to me, can you phrase it in another way?