747300 - Add mimetype info to nsIPluginTag

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: Patch v.0

Over in bug 619652 I'm looking at rolling about:plugins into about:addons. Currently the addon manager can show almost all the same stuff, so this largely means adding the plugin mimetype info. Unfortunately this doesn't seem to be exposed on nsIPluginTag, only nsIDOMPlugin. [It would be nice to combine these two interfaces someday, because they're virtually identical. Not going to do that here, though.]

How's this patch look? DOMMimeTypeImpl2 is a mostly cut-n-paste from DOMMimeTypeImpl over in nsPluginHost.cpp. Guess I should drop this into a shared .h file?

Comment 1

[Josh Aas]

(In reply to Justin Dolske [:Dolske] from comment #0)

I'm generally fine with doing this.

> Unfortunately this doesn't
> seem to be exposed on nsIPluginTag, only nsIDOMPlugin.

I'm not totally clear on why you don't just use the latter for this then?

Rest of the review coming soon.

Comment 2

[Josh Aas]

Comment on attachment 616877 [details] [diff] [review]
Patch v.0

Review of attachment 616877 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/base/nsIPluginTag.idl
@@ +49,5 @@
>    readonly attribute AUTF8String name;
>             attribute boolean  disabled;
>             attribute boolean  blocklisted;
> +  void getMimeTypes([optional] out unsigned long aCount,
> +                    [retval, array, size_is(aCount)] out nsIDOMMimeType aResults);

I'm not familiar with these idl argument options, but I assume you are.

::: dom/plugins/base/nsPluginTags.cpp
@@ +71,5 @@
>    return result;
>  }
>  
> +
> +class DOMMimeTypeImpl2 : public nsIDOMMimeType {

Yeah, lets not duplicate this code.

Comment 3

[Justin Dolske [:Dolske]]

(In reply to Josh Aas (Mozilla Corporation) from comment #1)
> > Unfortunately this doesn't
> > seem to be exposed on nsIPluginTag, only nsIDOMPlugin.
> 
> I'm not totally clear on why you don't just use the latter for this then?

Mostly just because the addons manager is already using the former. See buildPluginList() in toolkit/mozapps/extensions/PluginProvider.jsm.

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Fixed and de-bitrotted; it was working in the recent past but I haven't verified my bitrot fixes because m-c isn't compiling with gcc at the moment. :/

Comment 5

[Justin Dolske [:Dolske]]

Comment on attachment 654014 [details] [diff] [review]
Patch v.1

Frank says this does indeed build, so off for review!

Comment 6

[Frank Yan (:fryn)]

Actually, it just bitrotted again, due to patch "part 1" of bug 579517 landing.

Comment 7

[Frank Yan (:fryn)]

Attachment: patch v.1 (unbitrotted)

Unbitrotted patch.

Comment 8

[Frank Yan (:fryn)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52c224e20b26

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/52c224e20b26