Bug 57968

Summary: [GTK] Need a way to get the path for a WebKitWebPlugin
Product: WebKit Reporter: Gustavo Noronha (kov) <gustavo>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mrobinson, xan.lopez
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
how I plan to use this new API
none
patch addressing mrobinson's comments none

Description Gustavo Noronha (kov) 2011-04-06 12:06:33 PDT
I would like to only disable the flash plugin in Epiphany if it is not running through nspluginwrapper, but I need the plugin path to detect that =)
Comment 1 Gustavo Noronha (kov) 2011-04-06 12:09:17 PDT
Created attachment 88483 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2011-04-06 12:10:19 PDT
Created attachment 88484 [details]
how I plan to use this new API

Btw, we should make path, name, description actual GObject properties =)
Comment 3 Xan Lopez 2011-04-06 14:13:05 PDT
Comment on attachment 88483 [details]
Patch

There's not going to be 1.3.14. But looks fine to me otherwise. r +1/2
Comment 4 Martin Robinson 2011-04-06 14:25:41 PDT
Comment on attachment 88483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=88483&action=review

> Source/WebKit/gtk/webkit/webkitwebplugin.cpp:179
> + * Returns: the absolute path for @plugin.

Nit! "the absolute path for @plugin" --> "the absolute path to the @plugin"

> Source/WebKit/gtk/webkit/webkitwebplugin.cpp:192
> +    if (!priv->path.length())
> +        priv->path = priv->corePlugin->path().utf8();
> +
> +    return priv->path.data();

I think this should return the path in the filesystem encoding. For instance, g_fopen takes filenames in the system encoding. Either way the documentation should be clear about what encoding is returned.
Comment 5 Gustavo Noronha (kov) 2011-04-06 16:27:13 PDT
(In reply to comment #4)
> > Source/WebKit/gtk/webkit/webkitwebplugin.cpp:192
> > +    if (!priv->path.length())
> > +        priv->path = priv->corePlugin->path().utf8();
> > +
> > +    return priv->path.data();
> 
> I think this should return the path in the filesystem encoding. For instance, g_fopen takes filenames in the system encoding. Either way the documentation should be clear about what encoding is returned.

Well-thought! Do you guys agree to having this as part of 1.4.0? If so I'll address these comments and land it with Since: 1.4.0, and get it merged =)
Comment 6 Martin Robinson 2011-04-06 16:29:51 PDT
Seems like a safe addition to 1.4.0 to me.
Comment 7 Gustavo Noronha (kov) 2011-04-07 06:57:09 PDT
Created attachment 88624 [details]
patch addressing mrobinson's comments
Comment 8 Martin Robinson 2011-04-07 07:46:52 PDT
Comment on attachment 88624 [details]
patch addressing mrobinson's comments

View in context: https://bugs.webkit.org/attachment.cgi?id=88624&action=review

Great.

> Source/WebKit/gtk/webkit/webkitwebplugin.cpp:182
> + * Returns: the absolute path to @plugin in glib filename encoding.

s/glib/system. Should say it returns null on error.

> Source/WebKit/gtk/webkit/webkitwebplugin.cpp:206
> +    g_warning("Failed to convert '%s' to glib filename encoding: %s", priv->corePlugin->path().utf8().data(), error->message);

Ditto.
Comment 9 Gustavo Noronha (kov) 2011-04-07 10:44:58 PDT
Comment on attachment 88624 [details]
patch addressing mrobinson's comments

Landed in r83185