Add mimetype info to nsIPluginTag

RESOLVED FIXED in mozilla18

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

7 years ago
Posted patch Patch v.0 (obsolete) — Splinter Review
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?
Attachment #616877 - Flags: feedback?(joshmoz)

Comment 1

7 years ago
(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

7 years ago
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.
Attachment #616877 - Flags: feedback?(joshmoz) → feedback+
Assignee

Comment 3

7 years ago
(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.
Assignee

Comment 4

7 years ago
Posted patch Patch v.1 (obsolete) — Splinter Review
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. :/
Attachment #616877 - Attachment is obsolete: true
Assignee

Comment 5

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

Frank says this does indeed build, so off for review!
Attachment #654014 - Flags: review?(joshmoz)

Comment 6

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

Comment 7

7 years ago
Unbitrotted patch.
Attachment #654014 - Attachment is obsolete: true
Attachment #654014 - Flags: review?(joshmoz)
Attachment #655084 - Flags: review?(joshmoz)

Updated

7 years ago
Status: NEW → ASSIGNED

Updated

7 years ago
Attachment #655084 - Flags: review?(joshmoz) → review+
https://hg.mozilla.org/mozilla-central/rev/52c224e20b26
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.