Summary: | [GTK] Need a way to get the path for a WebKitWebPlugin | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||||||
Component: | WebKitGTK | Assignee: | 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
Gustavo Noronha (kov)
2011-04-06 12:06:33 PDT
Created attachment 88483 [details]
Patch
Created attachment 88484 [details]
how I plan to use this new API
Btw, we should make path, name, description actual GObject properties =)
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 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. (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 =) Seems like a safe addition to 1.4.0 to me. Created attachment 88624 [details]
patch addressing mrobinson's comments
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 on attachment 88624 [details] patch addressing mrobinson's comments Landed in r83185 |