Closed Bug 427743 Opened 13 years ago Closed 12 years ago

Expose File Version Number of Plugins

Categories

(Core :: Plug-ins, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.1a1

People

(Reporter: cbook, Assigned: mossop)

References

Details

(Whiteboard: [sg:want])

Attachments

(1 file, 3 obsolete files)

For Debugging of crashes and for blocklist Plugins we should expose File Versions of Plugins. I will also file a bug for displaying this information in about:plugins

<timelyx>	Tomcat, bug 423176 comment 16 has the hints for getting file version info
<timelyx>	Tomcat: see if you can find someone to write code for that
>timelyx>	it should be somewhere sharable
<timelyx>	since both plugins and accessible need it
Flags: wanted1.9.0.x?
Flags: blocking1.9?
Depends on: 427744
Blocks: 427744
No longer depends on: 427744
Summary: Expose Version of Plugins → Expose Filer Version Number of Plugins
This is available on windows onl as I understand it
OS: All → Windows 2000
Hardware: All → PC
Wouldn't block the release for something like this this late in the release cycle.
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9-
Summary: Expose Filer Version Number of Plugins → Expose File Version Number of Plugins
This will require interface changes so I'm not sure how we could add it in a point release. Question is is it wanted enough for 1.9.1 to take the interface change (nsIPluginTag)?

Could quite easily be done with an nsIPluginTag2 I guess.
Flags: wanted1.9.1?
Attached patch patch rev 1 (obsolete) — Splinter Review
This exposes the dll file version on nsIPluginTag as fileversion
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attached patch patch rev 2 (obsolete) — Splinter Review
bsmedberg says the interface change is ok on 1.9.1 as it is only additive.

This version adds support for reading the plugin bundle version on OSX when possible
Attachment #327125 - Attachment is obsolete: true
Attachment #327974 - Flags: superreview?(jst)
Attachment #327974 - Flags: review?(jst)
I should add that other platforms can add versions where available easily, I don't think Linux has any concept of it though and I haven't a clue about any other platforms.
Comment on attachment 327974 [details] [diff] [review]
patch rev 2

Looks good, but should we while we're here also go ahead an add a NP_GetPluginVersion() method for Unix plugins? Much like NP_GetMIMEDescription()?
Attachment #327974 - Flags: superreview?(jst)
Attachment #327974 - Flags: superreview+
Attachment #327974 - Flags: review?(jst)
Attachment #327974 - Flags: review+
Flags: wanted1.9.1? → wanted1.9.1+
(In reply to comment #7)
> (From update of attachment 327974 [details] [diff] [review])
> Looks good, but should we while we're here also go ahead an add a
> NP_GetPluginVersion() method for Unix plugins? Much like
> NP_GetMIMEDescription()?

Been looking at this, but I think in order to do that I would have to update nsIPlugin to add a method to retrieve a version, and wouldn't that break any plugins that were compiled as proper xpcom components? I'm not quite sure how many of those there are but it seems suggested that that is the recommended style now.
Attached patch patch rev 3 (test) (obsolete) — Splinter Review
This is a test patch that adds version lookups for OS/2 and BeOS. I have neither OS though so can't compile to test, will try to find someone who can.
Thanks, Dave, for digging up the OS/2 APIs for this kind of stuff. Will give your patch a try on OS/2. (I guess you will be out of luck to find someone to test BeOS as that only compiles on 1.8 branch.)
(In reply to comment #8)
> Been looking at this, but I think in order to do that I would have to update
> nsIPlugin to add a method to retrieve a version, and wouldn't that break any
> plugins that were compiled as proper xpcom components? I'm not quite sure how
> many of those there are but it seems suggested that that is the recommended
> style now.

nsIPlugin is going away in Mozilla 2. There's only one plugin that we know of that is an XPCOM plugin, and that's the Sun Java plugin, versions other than the newest one that ships with an NPRuntime enabled plugin. That new plugin is a pure NPAPI plugin as far as us talking to it goes (it internally still uses some XPCOM APIs, but that's unrelated to nsIPlugin).

So in other words, we should be able to add an NPP_GetPluginVersion() w/o touching nsIPlugin, as we don't care about plugin version information from plugins that are XPCOM plugins.
(In reply to comment #9)
> This is a test patch that adds version lookups for OS/2 and BeOS. I have
> neither OS though so can't compile to test, will try to find someone who can.

For OS/2 this has a few problems:
- it still contains |readOnlyString| instead of |version|
- NS_INFO_ProductVersion is actually saved with a length of 8 bytes
- version should be declared as a pointer of some kind, so that feeding its
  address to DosQueryResourceSize makes it a double pointer
With that it compiles, but the bit shifting to get the numbers doesn't work right. I haven't managed to find out how to correct it, though. Will let you know once I do.
Attached patch patch rev 4Splinter Review
This covers the main 3 platforms, adding Unix by attempting to find a NP_GetPluginVersion entry in the library. I've also renamed the property from fileversion to just version which seems to make more sense now it is no longer just a dll version.

I've been trying to figure out how to come up with a test for this. I could try to add a plugin that is only built when tests are enabled that a mochitest could them query a version of, but not sure if that goes against bug 443324. Any thoughts?
Attachment #327974 - Attachment is obsolete: true
Attachment #328856 - Attachment is obsolete: true
Attachment #329449 - Flags: superreview?(jst)
Attachment #329449 - Flags: review?(jst)
Comment on attachment 329449 [details] [diff] [review]
patch rev 4

- In a couple of places:

+ * NP_GetPluginVersion [optional]
+ *  - Netscape uses the return value to indicate to the user what version of
+ *    this plugin is installed.

No, Netscape doesn't use this version, but Mozilla does :) I know this just follows the same pattern in this code, but I'd deviate in this case and say either Mozilla, or "the browser".
Attachment #329449 - Flags: superreview?(jst)
Attachment #329449 - Flags: superreview+
Attachment #329449 - Flags: review?(jst)
Attachment #329449 - Flags: review+
Landed in changeset 0c080bbdb464
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
Depends on: 446159
Bug 391728 has tests for this
Flags: in-testsuite? → in-testsuite+
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080909020334 Minefield/3.1b1pre.   

Also added litmus test: https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7031
Flags: in-litmus+
Blocks: 454422
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1
Status: RESOLVED → VERIFIED
It's probably not realistic to get this into a 3.0.x release, unfortunately.
Flags: wanted1.9.0.x+
Whiteboard: [sg:want]
Depends on: 486946
You need to log in before you can comment on or make changes to this bug.