We can't load gtk2 plugin to gtk3 browser.

Does this work because gtk3 plugins don't import the symbol gtk_major_version but gtk2 plugins do? I wasn't aware that PR_FindSymbol worked for imported dynamic symbols: I thought it only worked for exported symbols.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #1) > Does this work because gtk3 plugins don't import the symbol > gtk_major_version but gtk2 plugins do? I wasn't aware that PR_FindSymbol > worked for imported dynamic symbols: I thought it only worked for exported > symbols. Yes, gtk_major_version is imported by gtk2 plug-ins from gtk libraries. For instance is listed as "unresolved" by flash plugin: $ nm -D /usr/lib64/mozilla/plugins/libflashplayer.so | grep gtk_major_version U gtk_major_version But I used it for gtk3/gtk3 plugin host switch and works as expected: handle = dlopen(aFilePath, RTLD_LOCAL|RTLD_LAZY); void *version = dlsym(handle, "gtk_major_version"); And the dlsym is also used by PR_FindSymbol: http://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/src/linking/prlink.c#1161

To be correct here the gtk_major_version is an integer, not function.

Comment on attachment 8333855 [details] [diff] [review] disable_gtk2_plugins.patch Review of attachment 8333855 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/PluginPRLibrary.h @@ +83,5 @@ > + /* We can't load gtk2 plug-in in gtk3 browser. > + * The gtk_major_version symbol is only in gtk2 library. */ > + if(PR_FindSymbol(mLibrary, "gtk_major_version")) > + return false; > +#endif This doesn't seem like the right place to be doing this check. Yeah, you're checking for a symbol, but it's not a required *NPAPI* symbol, which is the context for that code. Your issue has to do with whether or not it's a suitable library to use. I think 'nsPluginsDirUnix.cpp' is a better place for that stuff, can you move your check there?

Thanks, so something like that?

Comment on attachment 8341721 [details] [diff] [review] disable gtk2 plugins, v2 Review of attachment 8341721 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsPluginsDirUnix.cpp @@ +333,5 @@ > + * The gtk_major_version symbol is only in gtk2 library. */ > + if(PR_FindSymbol(pLibrary, "gtk_major_version")) { > + return NS_ERROR_FAILURE; > + } > +#endif Yes, but space between 'if' and '('. Did you test this, and it works as you want it to? How about with a pluginreg.dat file created by a gtk2 browser?

I doubt this actually works.

(In reply to Josh Aas (Mozilla Corporation) from comment #6) > Comment on attachment 8341721 [details] [diff] [review] > disable gtk2 plugins, v2 > > Review of attachment 8341721 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/plugins/base/nsPluginsDirUnix.cpp > @@ +333,5 @@ > > + * The gtk_major_version symbol is only in gtk2 library. */ > > + if(PR_FindSymbol(pLibrary, "gtk_major_version")) { > > + return NS_ERROR_FAILURE; > > + } > > +#endif > > Yes, but space between 'if' and '('. > > Did you test this, and it works as you want it to? How about with a > pluginreg.dat file created by a gtk2 browser? The PR_FindSymbol() check works, but GetPluginInfo() is called only when the plugin cache is updated for new plugins. OTOH If the check is in LoadPlugin() I have to close the library when the check fails, right?

Resolving old bugs which are likely not relevant any more, since NPAPI plugins are deprecated.