137535 - [Mach-O] support BP_GetSupportedMIMETypes so Quicktime will play MPEGs

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Peter Lubczynski]

This is an offshoot from bug 113464 where the extra entry points
BP_GetSupportedMIMETypes and NP_GetMIMEDescription started being supported on
Carbon builds for MPEGs to work with Quicktime. This bug is about porting those
changes to the Mach-O build (nsPluginsDirDarwin.cpp).

Comment 1

[Peter Lubczynski]

*** Bug 178422 has been marked as a duplicate of this bug. ***

Comment 2

[Simon Fraser [no longer active]]

We need this for chimera too. Anyone have a testcase?

Comment 3

[Peter Lubczynski]

see about:plugins, QT only lists 30 mime types in Mach-O beginning with
video/quicktime but CFM lists 45 types beginning with application/sdp.

The code that does this in the CFM build is here:
http://lxr.mozilla.org/mozilla/source/modules/plugin/base/src/nsPluginsDirMac.cpp#256

Comment 4

[Steve Dagley]

Attachment: Non functional Q&D first cut

I lifted the impl of GetPluginInfo() from nsPluginsDirMac.cpp and got it
compiling in nsPluginsDirDarwin.cpp but we're still not picking up the mime
types from the alternate entry point in the QT plugin.	I figured I go ahead
and post the patch in case it was a useful starting point.

Comment 5

[Peter Lubczynski]

Attachment: patch v.2 (to Chimera tree)

Here's the last patch but with the plugin version constant incremented to cause
us to re-scan all plugin on next launch. Which this patch I'm seeing the extra
mime types show up in about:plugins.

TODO:
- garbage text is showing up in the mime type fields in Apple's java plugin in
about:plugins 

TO TEST:
- turning off a mime type in the QT control panel gets refected in
about:plugins  (does plugin need to be stat'ed?)

Comment 6

[Peter Lubczynski]

Comment on attachment 106375 [details] [diff] [review]
patch v.2 (to Chimera tree)

Changing the mime types in the QT control panel isn't going to take effect
because we don't query the plugin if the timestamp doesn't change.  This is the
same problem as is already described with 'plugger' on Linux in bug 125469 and
also happens on CFM.

Opend bug 181010 about the garbled text. It needs to be fixed on CFM too.

Those bugs shouldn't hold up at least getting MPEGS working.

Comment 7

[Simon Fraser [no longer active]]

     printf("[loaded plugin %s]\n", path);
+    pLibrary = outLibrary;
     return NS_OK;

Please #ifdef DEBUG the printf.

The rest looks OK, but would be easier to read with a diff -w.

Comment 8

[Peter Lubczynski]

Attachment: patch v.2.1

updated patch

Comment 9

[Simon Fraser [no longer active]]

Looks good to me. If beard doesn't get to the SR, I can do it.

Comment 10

[Patrick C. Beard]

Comment on attachment 106872 [details] [diff] [review]
patch v.2.1

No need to initialize rv here:

+	   nsresult rv = NS_ERROR_FAILURE;
+	   rv = ParsePluginMimeDescription(pfnGetMimeDesc(), info);

In fact, you might as well declare rv at the top level of this function, since
it already seems to have an nsresult code.

Comment 11

[Patrick C. Beard]

Comment on attachment 106872 [details] [diff] [review]
patch v.2.1

Other than that, sr=beard.

Comment 12

[Peter Lubczynski]

patch in trunk, marking FIXED.

Simon, can you check this into the Chimera tree?

Comment 13

[Simon Fraser [no longer active]]

Will do.

Comment 14

[Simon Fraser [no longer active]]

I checked this patch into the Chimera branch.

Comment 15

[Chris Petersen]

Excellent.. Confirming video/mp4 is registered by the QT plugin and shows up in
the about:plugins window. Test case now shows the QT content.

http://bugzilla.mozilla.org/attachment.cgi?id=105206&action=view