[Webkit-unassigned] [Bug 39474] [GStreamer] GTK XOverlay support in GStreamerGWorld
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 2 15:44:04 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=39474
Gustavo Noronha (kov) <gns at gnome.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #62047|review? |review-
Flag| |
--- Comment #28 from Gustavo Noronha (kov) <gns at gnome.org> 2010-08-02 15:44:04 PST ---
(From update of attachment 62047)
WebKit/gtk/WebCoreSupport/FullscreenVideoController.cpp:571
+ g_object_set(m_exitFullscreenAction, "icon-name", "view-restore-symbolic", NULL);
symbolic icons are quite new, are we going to have to shift our dependencies for this?
294 if (!node->hasTagName(HTMLNames::videoTag))
295 return;
296
297 #if ENABLE(VIDEO)
Might as well only do the check if video is enabled? Otherwise it's just useless work.
311 void webkit_web_view_exit_fullscreen(WebKitWebView* webView)
312 {
313 #if ENABLE(VIDEO)
314 WebKitWebViewPrivate* priv = webView->priv;
315 if (priv->fullscreenVideoController)
316 priv->fullscreenVideoController->exitFullscreen();
317 #endif
Certainly a good idea to NULLify the fullscreenVideoController here. Also, I believe you need to exit fullscreen in case a fullscreenVideoController exists during WebKitWebView's destruction? The callbacks receiving C++ objects as user data still concern me, I'm pretty sure we'll get crashes from the object going away and the callback being called right after that.
These are my last concerns, I think, the rest of the patch looks good, so let's try to clear them up and land =D
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list