All non bundle plugins fail under OS X

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
Plug-ins
P1
critical
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Chris Petersen, Assigned: Steve Dagley)

Tracking

Trunk
mozilla0.9.3
PowerPC
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: OSX++, URL)

Attachments

(8 attachments)

(Reporter)

Description

17 years ago
Build: 2001050908
Platform: Mac OS X
Expected results: Carbonized OS X plugins like Quicktime, Shockwave, iTools
Plug-ins should be supported
What I got: Plugins are not functioning

Steps to to reproduce:

1) Go to a site that requires one of the three plugins like
http://www.apple.com/quicktime/

2) A dialog is displayed indicating a plugin is required.
(Reporter)

Comment 1

17 years ago
This is also a problem with our Java plugin. We simply don't render a applet
even thought the MRJPlugin is present.
Summary: Plugin support is not functioning → Third-party plugin support is not functioning
--> dagley
Assignee: av → sdagley
(Assignee)

Updated

17 years ago
Whiteboard: OSX+

Comment 3

17 years ago
What are the technical hurdles in getting plugins to work in OS X?
beta stopper
Whiteboard: OSX+ → OSX++

Comment 5

17 years ago
This Apple technote seemed oddly appropriate.

http://developer.apple.com/technotes/tn/tn2020.html

Hope it helps.

- Adam
(Assignee)

Comment 6

17 years ago
Yep, you can be sure someone at Netscape knows about this (X plugins used to be 
type IEPL :-)
Status: NEW → ASSIGNED

Comment 7

17 years ago
Plugins do work in Mac OS X. I made the Quicktime plugin work. You have to get a 
carbonized version of the Quicktime plugin, which I managed to find in:

/Library/Internet Plugin-ins/QuickTime Plugin.plugin

If you copy this file to our Plug-ins folder, Quicktime should work.
so is the only thing stopping this from working bug 78751? that would be great!
Depends on: 78751
(Reporter)

Comment 9

17 years ago
I can't get this to work in the latest Fizzilla (6/5) build. 

Comment 10

17 years ago
Oops, I believe that the QuickTime Plugin.plugin is in fact a bundle, not a 
single CFM binary. In an earlier release of Mac OS X, there was a CFM version 
bundled with the system... We probably need to write some further adapter code to 
load a plugin from a CFBundle. This isn't hard, but will require a bit more time.
(Assignee)

Comment 11

17 years ago
yeah, somebody at MacDev Monday (sfraser?) pointed out we'd need to handle this 
case.  I'm looking at it.
(Assignee)

Comment 12

17 years ago
This is on the list of OS X bugs that pink and I got PDT+'d but it never got 
marked as such.  Doing that now so I can extend that status to #78751 which this 
bug depends on.
Whiteboard: OSX++ → PDT+,OSX++

Comment 13

17 years ago
The MRJPlugin isn't Carbonized, because the JManager API isn't 
supported under Mac OS X. 

Comment 14

17 years ago
The shipping Mac OS X version of the QuickTime plugin *IS* a CFM 
plugin, but it is packaged as a Mac OS X bundle. I have developed some 
patches for the plugin module, and (gulp) NSPR to load Mac OS X 
bundles in addition to CFM shared libraries. With these patches in place, I 
can manually copy the file "/Library/Internet Plug-Ins/QuickTime 
Plugin.plugin" into the local Plug-ins folder.

Comment 15

17 years ago
Created attachment 39513 [details] [diff] [review]
Support loading Mac OS X bundle packaged plugins.

Comment 16

17 years ago
Adding wtc to get his take on the NSPR changes. These patches have 
some debugging printfs and some #if 0'd out code. I'll clean those up in a 
subsequent patch.

Comment 17

17 years ago
Oh that's great about Quicktime! With this patch:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=39206

from bug 78751, you shouldn't need to copy.
comments on the patch:
- add a comment to getPluginBundle() and getLibraryBundle() that the caller is
responsible for freeing the returned reference with ::CFRelease().
- check for 'BRPL' in addition to 'NSPL'
- can these bundles exist on OS9 as well? Does this code have to be target_carbon?
- either remove or #if DEBUG the printf's
- you leak the bundle returned from getLibraryBundle() in prlink.c

--> beard, who is working on this actively
Assignee: sdagley → beard
Severity: major → critical
Status: ASSIGNED → NEW
Keywords: nsBranch
Target Milestone: --- → mozilla0.9.2

Comment 19

17 years ago
It doesn't hurt to call the CFBundle code under all versions of CarbonLib. I 
have fixed all of the issues you raise in the next patch.

Comment 20

17 years ago
Created attachment 39654 [details] [diff] [review]
cleaned up patches

Comment 21

17 years ago
Created attachment 39655 [details] [diff] [review]
Oops, last patch got garbled by IE...

Updated

17 years ago
Whiteboard: PDT+,OSX++ → PDT+,OSX++; patch available

Comment 22

17 years ago
Patrick, I reviewed your patch for prlink.c.  It is fine.
Could you remove the two fprintf() statements?  At least,
comment them out with #if 0.

Comment 23

17 years ago
They are #ifdef DEBUG, but I will remove the one for every function pointer 
lookup. Hearing about a bundle loading in DEBUG builds is probably good for now.
patch #3 looks good, r=pink

Comment 25

17 years ago
Created attachment 40088 [details] [diff] [review]
Final cleanup of changes to prlink.c per wtc's comments.

Comment 26

17 years ago
I reviewed the patch for prlink.c.  It is fine.

In general we don't allow fprintf in NSPR, even in the debug build.
A better solution is to use PR_LOG.  But I'll make an exception.

r=wtc.

Updated

17 years ago
Whiteboard: PDT+,OSX++; patch available → PDT+,OSX++; patch available, need approval

Comment 27

17 years ago
I can certainly rs=, but sfraser would probably be a better person to look at 
this.

Comment 28

17 years ago
Comments:

+    if (err == noErr && FSRefMakePath) {
Please make the weak-link test for FSRefMakePath more explicit, like:
  (long)FSRefMakePath != kUnresolvedCFragSymbolAddress

+            CFStringRef pathRef = CFStringCreateWithCString(NULL, path, 
kCFStringEncodingMacRoman);

Use kCFStringEncodingUTF8 (since FSRefMakePath returns a UTF-8 path, per 
headers).

+        Boolean hasExecutable = CFBundleLoadExecutable(bundle);

This seems expensive. Consider CFBundleGetPackageInfo() instead, checking the 
bundle bit (maybe using PBGetCatInfo).


NSPR changes:

+    if (err == noErr && FSRefMakePath) {
ditto
+            CFStringRef pathRef = CFStringCreateWithCString(NULL, path, 
kCFStringEncodingMacRoman);
ditto

Fix those, and sr=sfraser

Updated

17 years ago
Status: NEW → ASSIGNED
Priority: -- → P1

Comment 29

17 years ago
Created attachment 40215 [details] [diff] [review]
Cleaned up patch with sfraser's feedback incorporated.

Comment 30

17 years ago
Will check into trunk tonight.

Comment 31

17 years ago
Checked into trunk.

Comment 32

17 years ago
Patrick, please be sure to check in the patch for prlink.c
on both the trunk and NSPRPUB_CLIENT_BRANCH of NSPR.  Thanks!

Updated

17 years ago
Whiteboard: PDT+,OSX++; patch available, need approval → PDT+,OSX++; checked into trunk, need peer review

Comment 33

17 years ago
Checked prlink.c changes into NSPR tip.

Comment 34

17 years ago
All checkins done. Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 35

17 years ago
I checked this with the latest Mac branch (2001062703) under Mac OS X 10.0.4.
This build , which runs in Classic (OS 9) mode, will recognize and load the
Quicktime plugin located at the /Library/Internet Plugin-ins/QuickTime
Plugin.plugin. However, other plugs-in at this same location (Shockwave Flash NP-PPC
iTools Plug-in) are not loaded.
(Reporter)

Comment 36

17 years ago
I still need to check in a newer build of  "Fizzila" to see if the Quicktime
plug is working.
(Reporter)

Comment 37

17 years ago
So based on my test results with other plug-in than Quicktime, should I expect
these plug-ins to be loaded ?  Or should I close out this bug since QT works and
create new bugs on these plug-ins that fail ?

Comment 38

17 years ago
Please open a new bug to deal with the classic build plugin problems. This bug 
was only meant to address the problem of loading internet plugins that are 
packaged in the new Mac OS X "package" format (a directory with its bundle bit 
set containing a plugin in various pieces). As I said in a phone conversation 
with Chris, the classic build is unaffected by the checkins made to fix this bug. 
The classic build will only consider additional plugins stored in the folder:

"{ClassVolume}System Folder:Internet Plugins:"

The carbon build will only consider additional plugins stored in:

/Library/Internet Plugins
(Reporter)

Comment 39

17 years ago
Thanks for the info, Patrick. If I move non-carbon
plug-ins (itools and shockwave) into this Internet Plug-in (System 9) folder, N6
will find and load them. For this fix , I will check on the next fizzilla build. 

Comment 40

17 years ago
Platform: PowerBook G3/300/192Mb/25Gb, MacOS X 10.0.4
Fizzilla build: 20010702 (0.9.2)

I do not believe this is fixed. I just moved a copy of  /Library/Internet
Plug-Ins/QuickTime Plugin.plugin to /Applications/Mozilla 7/2/Plug-Ins/ and
relaunched Moz. (Whether the name of the folder is "Mozilla 7/2" or "viewer"
makes no difference.) When I go to the Help menu, About Plug-ins, I get a
message that "No plug-ins are installed." If I go to Apple's Quicktime web page

http://www.apple.com/quicktime/

the right hand bar (drawn by the QuickTime plug-in) isn't there; instead, I get
a message saying, "Alert! QuickTime is not properly installed."

Something isn't right here...

- Adam
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
can you try the 7/3 branch build? i don't know if beard got these fixes in for 
0.9.2.

Comment 42

17 years ago
I downloaded the 7/3 build. Unfortunately, I've still got the same problem. Made
a copy of the QuickTime plugin into the new Fizzilla's Plug-Ins folder, and I
still have the same problems I previously reported.

- Adam

Comment 43

17 years ago
looking for help on this to investigate until beard returns..
removed old status whiteboard comments.
Whiteboard: PDT+,OSX++; checked into trunk, need peer review → PDT+,OSX++;
(Assignee)

Comment 44

17 years ago
Taking bug.  Will investigate once I finish building you know what for you know 
who.
Assignee: beard → sdagley
Status: REOPENED → NEW

Updated

17 years ago
Blocks: 88870
(Assignee)

Comment 45

17 years ago
Changing resolution on this one back to FIXED as QT is now working again
Status: NEW → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 46

17 years ago
Chris Petersen and I were just talking about this bug and I mentioned that Flash
still doesn't seem to be working in OS X even though the same plugin works in IE.

Should we open another bug for other plugins besides Quicktime or use this one?
(Reporter)

Comment 47

17 years ago
With the Mozilla July 10th branch build (2001071013), QT plug-in is recognized
and appears in about: plugins window. QT movies are rendered in the browser
window. The shockwave plugin , which is displayed in the about: plugins window,
is not working when viewing flash content. Should we close this bug with the QT
plug-in and open a new one for shockwave ?
(Reporter)

Comment 48

17 years ago
Clarification:

When I refer to the shockwave plugin, I mean the flash plugin which is stored at
Library/Internet Plug-in/Shockwave Flash NP-PPC

Also, IE 5.5 works with the Shockwave Flash NP-PPC.

Comment 49

17 years ago
Personally, I think this bug should refer to the bundled plugins with OS X. If
Flash isn't working, then it should be reopened. If the bundled plugins don't
work, then something is still broken.

- Adam

Comment 50

17 years ago
Okay, I just took a look in the code and it looks like Flash (and probably other
Carbon plugins) are failing when we try to load them in NSPR. Looking in
prlink.c,  pr_LoadLibraryByPathname() calls getLibraryBundle() which fails for
any plugin except Quicktime. Looks like there are several ways
getLibraryBundle() can fail, so I guess it's time to break open the debugger.

w.r.t. this bug, it doesn't matter to me if another bug is opened for Flash (and
other plugins). On the other hand, it looks like the problem is directly related
to the code in the patches in this bug. Not caused by it, but rather just
exposed. Maybe someone with more Carbon experience can take a look at
getLibraryBundle() and see if there is anything obvious:
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/linking/prlink.c#491

Comment 51

17 years ago
changing 'qa-> Chris'
QA Contact: shrir → petersen
(Reporter)

Comment 52

17 years ago
Reopening bug and changing description.

The terminal window directory listing shows QT plug-in as a directory and Flash
plugin is shown as a file.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Third-party plugin support is not functioning → All non bundle plugins fail under OS X
(Reporter)

Comment 53

17 years ago
*** Bug 90248 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 54

17 years ago
Some notes on the subject...  On my 10.0.4 install the 'Shockwave Flash NP-PPC' 
plugin has a file type of 'BRPL' which we do handle in nsPluginsDirMac.cpp.  The 
'iTools Plug-in' plugin has a type of 'IEPL' which we don't handle unless it is a 
package (it is not in this case).
Status: REOPENED → ASSIGNED

Comment 55

17 years ago
any updates on this bug?
(Assignee)

Comment 56

17 years ago
This bug is Mac OS X specific and does not affect 'Classic' builds for Mac OS 
8.6/9.x

Comment 57

17 years ago
Steve, could you update the status whiteboard with your "ETA="?

Comment 58

17 years ago
Removing PDT+ based on sdagley comment of 7/12.
Whiteboard: PDT+,OSX++; → OSX++;

Updated

17 years ago
Whiteboard: OSX++; → OSX++; take fix if available before last spin to unblock OSX. pdt+

Comment 59

17 years ago
I found the problem with non-bundle plugins. We rely on being able to distinguish 
between modern plugins, carbon plugins and classic plugins by looking up various 
exported entry points, namely "NSGetFactory" for modern plugins, "main" for 
carbon plugins, and "mainRD" for classic plugins.  I used DumpPEF on both the 
"Shockwave Flash NP-PPC" plugin and the "iTools Plug-in" that are installed on 
Mac OS X systems in "/Library/Internet Plug-Ins", and neither of them exports a 
"main" entry point. The iTools plugin in fact exports no symbols at all. However, 
both of them do provide a CFM main entry point which is returned to NSPR when it 
calls GetSharedLibrary/GetDiskFragment in pr_LoadLibraryByPathname(). 
Unfortunately, we have no way to get at that main CFM entry point in the current 
NSPR.

To verify that this is the correct main to call, I patched prlink.c to keep a 
pointer to the CFM main entry point in the PRLibrary struct, and to it return if 
a program calls PR_FindSymbol(library, "main"). This works, but I'm not happy 
about fixing it this way. Any other suggestions? I'll attach the patch.

Comment 60

17 years ago
Created attachment 42723 [details] [diff] [review]
patch to prlink.c to provide a way to access CFM main entry point

Comment 61

17 years ago
I also noticed that we aren't building the "pluginClassic.mcp" project for Carbon 
builds running on Mac OS 9, which would prevent us from running all legacy 
classic plugins. I'm attaching a patch to the build scripts that turn that on.

Comment 62

17 years ago
Created attachment 42724 [details] [diff] [review]
enable building pluginClassic.mcp for carbonized builds.
as i told beard, i'd prefer an API to get the main entry point, but this is OK 
for the branch. Are there any problems with looking for 'main'? What if there's 
already a symbol with that name that's not what we want? (perhaps that can't 
happen). we can always add the api on the trunk, let's get this onto the branch.

r=pink, sr=scc.

who from nspr-land needs to approve this? gordon? wtc?

Comment 64

17 years ago
It's possible the morning's build is that last spin you're predicated on.  Is
this actually ready to go in?  (Looks like it might still be under discussion.)

Comment 65

17 years ago
I still need feedback/approval from wtc regarding this patch. We probably 
shouldn't be adding an API at this stage, so the issue is really whether we 
should add this to all builds, or perhaps just the TARGET_CARBON builds.

Comment 66

17 years ago
Checking this change into the 092 branch only for now. Otherwise, we won't be 
able to support the flash plugin.

Comment 67

17 years ago
Patrick,

Your last patch to prlink.c (attachment id=42723) is fine because
it does not change NSPR's public API.  Since pinkerton and scc
already reviewed the patch, you can go ahead and check it in on the
NSPRPUB_CLIENT_BRANCH and trunk of NSPR.

When you check it in on the trunk of NSPR, please apply the patch
as opposed to just copying the prlink.c file from NSPRPUB_CLIENT_BRANCH.
This is because the prlink.c file on the trunk has some new functions.

Comment 68

17 years ago
We're leaving the PDT+ on this still on the assumption that you'll only check in
the fix if it's completely OSX specific and can't have any interaction with
non-OSX builds.
Whiteboard: OSX++; take fix if available before last spin to unblock OSX. pdt+ → OSX++ (on branch, needs to go on trunk)

Comment 69

17 years ago
PDT+ for sdagley to complete the checkin on the branch.
Whiteboard: OSX++ (on branch, needs to go on trunk) → OSX++ (on branch, needs to go on trunk) PDT+ for sdagley's update
(Assignee)

Comment 70

17 years ago
Created attachment 43108 [details] [diff] [review]
Patch to recognize non-bundle plugins of type 'IEPL' under OS X
(Assignee)

Comment 71

17 years ago
Attached patch to recognize non-bundle plugins of type 'IEPL' under OS X (e.g. 
Apple's iTools plugin), landed same on 0.9.2 branch, removed PDT+ since it's 
checked in.  Still open until landed on trunk.
Whiteboard: OSX++ (on branch, needs to go on trunk) PDT+ for sdagley's update → OSX++ (on branch, needs to go on trunk)

Comment 72

17 years ago
Adding vbranch keyword for QA.
Keywords: nsBranch → vbranch
Whiteboard: OSX++ (on branch, needs to go on trunk) → OSX++ (fixed on branch, needs to go on trunk)
(Reporter)

Comment 73

17 years ago
With the July 21 (2001072103) branch build, I can verify that non bundle plugins
such as flash and itools are loaded and functional.

Updated

17 years ago
Target Milestone: mozilla0.9.2 → mozilla0.9.3
(Assignee)

Comment 74

17 years ago
Fix for 'IEPL' plugin type is now checked in to trunk, when beard checks in the 
prlink.c this bug can be closed
(Assignee)

Comment 75

17 years ago
beard's patch to prlink.c checked in, marking FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 76

17 years ago
Verified on trunk build (2001-08-04-05)
Status: RESOLVED → VERIFIED
Keywords: vbranch
Whiteboard: OSX++ (fixed on branch, needs to go on trunk) → OSX++
You need to log in before you can comment on or make changes to this bug.