[Webkit-unassigned] [Bug 76181] [GTK] FullScreen signals

[bugzilla-daemon at webkit.org]

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


Xan Lopez <xan.lopez at gmail.com> changed:

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




--- Comment #7 from Xan Lopez <xan.lopez at gmail.com>  2012-02-02 01:49:49 PST ---
(From update of attachment 123118)
View in context: https://bugs.webkit.org/attachment.cgi?id=123118&action=review

r- at least for the accumulators used.

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:932
> +        return;

If the handler handled the signal it will return TRUE, so I still think if you need to flip the boolean here. Perhaps this is not working correctly because you are using the wrong accumulator.

> Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:951
> +    g_signal_emit_by_name(m_webView, "leaving-fullscreen", &returnValue);

You need to check the return value and do nothing if it's TRUE, per what the documentation says.

> Source/WebKit/gtk/tests/testwebview.c:478
> +    g_test_add_data_func("/webkit/webview/fullscreen", (gconstpointer) TRUE, test_webkit_web_view_fullscreen);

No spaces before the cast?

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2638
> +     *

FALSE to block the event? Usually TRUE = hanlded ("block the event"), FALSE = continue emission. So I think you are saying the same thing twice basically.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2646
> +                         g_signal_accumulator_first_wins, 0,

I think you want true_handled, not first_wins.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2670
> +                         g_signal_accumulator_first_wins, 0,

Same.

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