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

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Philippe Normand <pnormand at igalia.com>  2012-01-19 04:18:05 PST ---
(In reply to comment #4)
> (From update of attachment 122247 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122247&action=review
> 
> An overall comment, I think it would fit using the policy action mechanism here.
> 
> > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:932
> > +        return;
> 
> Shouldn't this be if (returnValue) ? Otherwise you are doing nothing if the signal is not handled? Maybe I'm on crack.
> 

I don't think so. returnValue would be FALSE here if the signal wasn't handled properly.

> > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:951
> > +    g_signal_emit_by_name(m_webView, "leaving-fullscreen", &returnValue);
> 
> Shouldn't this also allow to cancel the action?
> 

I haven't found a use-case for this but yeah, I'd be Ok with allowing to cancel the action.

> > Source/WebKit/gtk/webkit/webkitwebview.cpp:2622
> > +    /**
> 
> Missing Since: in the signals?
> 
> > Source/WebKit/gtk/webkit/webkitwebview.h:180
> > +    gboolean                   (* leaving_fullscreen) (WebKitWebView   *web_view);
> 
> You need to remove padding from the class, otherwise you break the ABI.

Ok!

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