Closed Bug 90959 Opened 19 years ago Closed 19 years ago

Plugins may not work at all on less than OS 9.1

Categories

(Core :: Plug-ins, defect, P1, critical)

PowerPC
Mac System 8.6
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.3

People

(Reporter: tracy, Assigned: peterlubczynski-bugs)

References

()

Details

(Keywords: regression, Whiteboard: PDT+)

Attachments

(6 files)

Seen on recent builds, trunk and branch.

-goto about:plugins to be certain that the flash player is present.

-goto the listed site to view a flash page. click on flash version

expected results: An animated Flash page should open and be in motion. 

tested results: the window where the Flash animation is supposed to view is 
blank.
OS: Windows 98 → Mac System 9.x
Hardware: PC → Macintosh
On Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:0.9.2+) Gecko/20010713 with MacOS
8.5 about:plugins shows the flash plugin loaded but when you go to any flash
site the plugin doesn't starts the animation saying that does not have a plugin
registered.
Investigating.......this is a regression as it works on my debug build from last
week but fails in this morning's bits from sweetlou. I just nuked my tree and am
starting a fresh pull/build of the branch.

Not all flash pages are effected. http://www.hardrockhotel.com worked for me. It
could be HTML related but that still doesn't explain why it worked in last
week's builds. The problem here is that the default plugin gets activated
instead of flash but Flash is indeed picked up in About:plugis, it's there and
works on other pages. 

Tested on the same build on Win32 and it works flawlessly.
Assignee: av → peterlubczynski
Keywords: regression
Priority: -- → P1
Summary: Flash player not working on Mac → Flash player not working on Mac on some pages
Target Milestone: --- → mozilla0.9.3
I went to http://www.disney.com. It totally failed to bring up the flash
animation. I did an about:plugins to insure Flash was installed. Set some
breakpoints, went back to disney... and it loaded and ran. Now it runs the Flash
animation everytime I go there...even after quitting and re-launching... wierd.
Brian,
I'm still building but try clearing your disk cache. Or, another weird thing I
saw is that that going to about:plugins causes a plugins.refresh() which changes
things.  What happens if you go straight there?
I'll try the disk cache. After re-launching, I went straight there and it worked
correctly.

I just went from one page (disney main) to another (sub page) and got the "you
need to get the flash plugin" message. After that, no more flash...

It does appear that either the default plugin is taking precedence, or the
mimetype test is "missing" sometimes...
Could this be related to bug 90152? The patch in that bug is around code that
could effect this.
Status: NEW → ASSIGNED
Clearing disk cache doesn't seem to have any effect. Currently, going to disney
from product launch, I get the functional page, click on any link, get broken
plugin image, go back to main page, get dialog that I don't have the plugin
finder. No more Flash until I quit and relaunch.

It's almost like the mimetypes get removed from the list after the first time
they are accessed.
I applied the patch from bug 90152 with no effect.
Just tried on Chris Petersen's machine with similar results:

1) Launch browser
2) Visiting http://www.hardrockhotelcom works
3) Then go to http://www.disney.com and it doesn't

Reverse #2 and #3 above and the second Flash page you visit doesn't work.

I'm <still> building but you can visit this page:
http://praline.netscape.com/plugins/manager3.html
That page goes through the navigator.plugins[] array without first doing a
plugins.refresh()

Hm...looing at CVS log, I'm now maybe suspecting my (1.275) checkin to
nsPluginHostImpl.cpp to make mime type comparison case-insensitive.
Summary: Flash player not working on Mac on some pages → Flash stops working after first page on Mac
Hmm, those changes look pretty safe. I think I'd be more suspicious of the
LoadPlugin changes you made (don't remember the bug #, just recall thinking that
that code was pretty touchy and prone to breakage.)
It looks like the problem starts in nsObjectFrame::Reflow() where we we seem to
be falling into the code commented "Otherwise we're either an ActiveX control or
an internal widget..."

IsPluginEnabledForType is getting called with a mimetype of either
"application/oleobject" or "application/x-oleobject".
Brian,

I find the problem much deeper than that. If you set a breakpoint in
nsPluginHostImpl::GetPluginFactory(), you'll notice that about halfway through
we try to call nsPluginFile.LoadPlugin() which in turn causes PR_LoadLibrary to
be called in nsPluginDirMac.cpp.

The problem seems to be that PR_LoadLibrary() fails. Now for the hard part, why
does this fail?
Yeah, I had discovered that the oleobject thing was a red herring. It just
happened to be the first thing I came across that seemed odd.
It has the wrong path. I'm currently trying to open the library
"...mozilla:dist:viewer_debug:Shockwave Flash NP-PPC". Shockwave is in my
internet plugins folder.
Hm....I don't think that's it as I my System Folder:Internet Plug-ins folder is
empty and it still fails in PR_LoadLibrary with the correct path to the correct
plugin.

Clearing the regression keyword as I think you may have exposed this bug when
you fixed the nsITimer. That was probably broken for a long time and I bet
normal shutdown as on Windows was never being called on the Mac. We are probably
doing something wrong there, but I verify that TryUnloadPlugin() is being
called. In there we call Shutdown() on the plugin and also call
PR_UnLoadLibrary(). Btw, comenting out the library unload does not fix this.
Keywords: regression
You didn't not what your path is, but notice that mine would be wrong even if I
had the plugin in the application folder... there is no "Plug-ins" in that path
(or "Components" for that matter...
Keywords: regression
That should have said "You didn't note". You do appear to be correct about the
timer changes uncovering this... If you comment out the mPluginTimer = nsnull;
line in CancelTimer() the problem does go away. Something we are doing in the
nsPluginInstanceOwner destructor is causing this.
I'm on to something... In nsPluginHostImpl::StopPluginInstance() (line 4648) the
code checks to see if it is a "oldschool" (4.x) plugin. Right below this is a
check that does:

if (!doCache || oldSchool)
changing this to
if (!doCache || !oldSchool)
fixes the problem... But this logic wrong, or is it just another symptom?
I think that may just be a byproduct. If you change that, Shutdown() on the
plugin won't ever be called until we exit the browser. According to the spec,
it's supposed to be called when the last instance is going away.
Ahh, Brian, you were totally right about the path being incorrect....

Taking a closer look, it does indeed look like the call to PR_LoadLibrary() is
missing the "plug-ins" folder in the path. It goes straight from viewer_debug to
Shockwave. Looks like pluginTag->mFileName is in correct, as far as I can tell.

I think the problem may be in nsPluginDirMac.cpp, I'll take another peek tonight.
Nope, I see the problem now. I knew this was going to bite us sooner or later
and I guess fixing the leaking nsITimer exposed the problem.

On Windows (and on others I think), pluginTag->mFileName stores a full path to
the filename of the plugin. On, Mac this does exactly what it says, it stores
JUST the  filename. So, after we've gotten this plugin once, we save the
filename so that later we don't have to find it. I think
ns4xPlugin::SetPluginRefNum() may do that, however, I think it would require a
big hack in nsPluginHostImpl::GetPluginFactory() to use it. I think it would be
easier to fix GetPluginInfo() in nsPluginsDirMac.cpp to fully resolve fFilename.
On the flash testcases I tried tonight, that patch seems to fix it by using the
full path. Since the code is already expecting this on other platforms, it seems
to work. I did have some problems with Quicktime.

One interesting note though, I was crashing on the first call to
::CloseResFile().  It's probably not a good idea, but I just comented it out for
now.
Keywords: patch
Summary: Flash stops working after first page on Mac → Plugin completely stop working after first use on Mac
Whiteboard: PDT
Attached patch better patch v.2Splinter Review
I think that last patch should do the trick and should be very low risk.
Quicktime and Flash both work and keep working.
Peter, what is this mFullPath -- does it contain the leaf name as well? Also, 
haven't you miss one of the nsPluginTag constructors to initialize this new 
member?
This looks pretty serious.  I will email PDT for PDT+ with a cc: to peter and
aruner.
Removing the CloseResFile call will cause us to keep the resource fork of the
plugin open for the lifetime of the Mozilla instance. This would be bad. That
close corresponds to the FSpOpenResFile in the constructor. The other open/close
pair is in the plugin directory scanning code which only happens at startup.

Oddly enough, I didn't have any trouble related to the CloseResFile call you
removed. There must be something else at play here...

If you are going to initialize mFullPath to nsnull in the third constructor, you
need to validate it before using it in GetPluginFactory.

You are going to leak mFullPath in FreePluginInfo.
Is there a good justification for having both mFileName and mFullPath? Why not 
just to add the path to mFileName and keep a parity among platforms?
Can't do it that way. Much of the underlying code on the mac expects to get just
the leaf name.
A better way to state that would be:

Much of the underlying Mac code expects just the file name. It could be changed,
but it would probably have far reaching consequences (based on what I saw when I
looked).
Brian is right, we can't make this to parity with the otherplatforms because the
Mac expects just the leaf name. Some plugins have problems.

So the ::CloseResFile call isn't giving you problems? Hm...I'll try it again and
also fix up FreePluginInfo so not to leak a mFullPath.

Lisa, yes, I think this is a stop-ship bug and would like PDT+ approval.
PDT+ per PDT email for branch.  Trunk checkins do not need PDT approval.
Whiteboard: PDT → [PDT+]
Changes look good... I'm concerned about one thing though. Back to that third
constructor again...

That is the constructor that is used by LoadXPCOMPlugin(). It looks to me like
this patch will completely disable XPCOM plugins because mFullPath will be null
and therefore GetPluginFactory will always return NS_ERROR_FAILURE.

Either the filename should be used here (on the assumption that it worked
before) or the full path param should be passed to the constructor so it can be
set. (But then you won't have to check for null anymore :).)
Attached patch new patch, v.4Splinter Review
Okay, v.4 now takes care of the XPCOM case. It actually turns out that XPCOM
plugins were doing this correctly anyway and I confirm that the filename being
passed to the nsPluginTag constructor is the full and correct path to the plugin
(in the components directory). Since the filename is already being set
correctly, I simply set mFullPath to be the same as mFileName.

Can I get your r=? Thanks.
Why not to add mFullPath = nsnull; in the default constructor too?
r=av
sr=attinasi
Please land this puppy on the *branch* right away!  Thanks for the fix!!
*** Bug 90464 has been marked as a duplicate of this bug. ***
Fix is on the BRANCH!!!

I'll try to get it into the trunk as soon as I can.
Summary: Plugin completely stop working after first use on Mac → 90959
Whiteboard: [PDT+] → [PDT+][patch on the branch]
Summary: 90959 → Plugin completely stop working after first use on Mac
Keywords: vbranch
Whiteboard: [PDT+][patch on the branch] → patch on the branch
Patch on both the trunk and the branch now, marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: vbranch
Resolution: --- → FIXED
Whiteboard: patch on the branch → PDT+
did the fix for the branch make it in on time to show on the 2001-07-18-03-0.9.2 
build?  I am still seeing this on that build.
Tracy,

I just downloaded Mac Classic 2001071803 mozilla.org bits and the above URL (and
others) work fo me. I will try the commercial ones off sweetlou now.
sorry, that WAS the 0.9.2 I downloaded from mozilla.org, not the trunk (but 8am
trunk should have it).
Tracy, WFM on Mac 2001-07-18-03-0.9.2 at www.disney.com. Where are you seeing
the problem occur?
the smoketest points me to http://www.tombraider.com/flash.html... there I click 
on flash version
I see no problems there either... something wierd here.
I see no problems with http://www.tombraider.com/flash.html
 on both the commercial and mozilla branch 3 am builds. 

Tracy, what URL did you download your build from?
tried today's mac brnch build and this works now..tried a priliminary test and 
this test url...*calling Tracy now*
i retrieved the bits for the smoketests from sweetlou.

also, shrirang just dropped by my cube to confirm the bug.....he's going to 
check a few more mac machines...in the meantime I am rebooting to see if that 
helps and then I will get a build without the installer to see if that makes any 
differnece.
I'm using the 2001-07-18-03-0.9.2. Problem has been resolved for me too. Tested
under Mac OS 9.1.
i could reproduce what Tracy said...*BUT that was using 0717 branch* 
(yesterday's build). Using today's build with this fixed...this is definitely 
fixed. tried a mac OS 9.1 and OS 8.6 and 9.0.4. Tracy, pls try reinstalling in a 
different location or something, let me know.
nope, 8.6 IS showing this problem on today's mac branch (0718). Petersen can u 
pls reconfirm? reopening, changing OS.
Status: RESOLVED → REOPENED
OS: Mac System 9.x → Mac System 8.6
Resolution: FIXED → ---
Yes, confirming problem is still occuring under 8.6 with the 7/18 branch build.
Dialog appears which states plug-in required.
Chris, do you mean "It works the first time and the second time the dialog
appears which states plug-in required" or you get a dialog the first time you
try to go to the page?
The first time I go to the page I get the dialog.
That would imply that the plugin is simply failing to load under 8.6 then. Are
we sure that the plugin supports 8.6?
BTW, The flash plug-in does show up in the about:plug-ins window.
The machine I am seeing this on is a Mac 9.0.4 
brian: yeah, flash used to work with Mac OS 8.6 (last tried on this OS was..last 
month or so)
Yeah, it probably will. If the plugin is in the plugins directory the directory
scan will pull in the information that is used by about:plugins. This does not,
however, attempt to instantiate an instance of the plugin. I'm guessing that the
failure to instantiate the plugin is somehow causing the default plugin to be
triggered. Of course, I'm only guessing...
Ok, if it used to work, then maybe we've somehow broken plug'ins on 8.6? Can you
try some other plugins and see what happens?
Just saw Tracy's response (why do our mail servers send stuff out of sync?). So,
this ONLY works on 9.1?
i have a 9.0 and this works there too
I've been struggling with getting other plugins to download.  I used 4.x on 
shrirangs andvice and was able to get Quicktime loaded.  It works with the same 
build that was failing the Flash test.  But a new development has arised.  The 
flash player worked for one time at the tombraider site.  Returning there and it 
asks to "click here after downloading the plugin." Then going to Disney.com 
gives me the the downloader pluging download message. Then upon returning to the 
tombraider site the viewing area is blank...no alerts, no puzzle piece.
Allright! Now *that's* the bug that should have been fixed by this check in.
Do we have a patch that's ready for the branch?  The next build may be the last...
I believe this bug is now different than it's orriginal one, and here's the scoup:

This doesn't seem to fail only any OS 9.1 machine so yesterday I tried to set up
a debug envornment and build on an old G3 that I set up with OS 8.6. It should
be complete by the time I get into the office, ready to debug. Hopefully the
problem is not hard to fix.

Shrirang, have you been able to determine why this happens on Tracy's OS 9.0 and
not yours? I'm wondering if it's an certain extension or version of an extension
that's no updated. Perhaps it's CarbonLib.
Status: REOPENED → ASSIGNED
Keywords: patch, regression
Summary: Plugin completely stop working after first use on Mac → Plugins may not work at all on less than OS 9.1
Whiteboard: PDT+ → PDT+[ETA Thursday evening]
The problem is that on OS 8.6 and some OS 9's, there is no Internet Plugins 
Folder in the System Folder or it's not supported and this was an error condition 
in the code. In the patch attached, I've moved the check to below the scan of the 
app's local plugins folder, and bail if we didn't find anything THEN.

I'm seeking reviews for this patch.
Keywords: patch, regression
Whiteboard: PDT+[ETA Thursday evening] → PDT+[SEEKING REVIEWS]
Won't that cause you to attempt to iterate on a bad "pluginsDir"? I think all
you want to do is skip the first loop if "pluginsDir" isn't valid.
That's more like what I had in mind :).
r=bnesse.
sr=attinasi
...waiting for clearence to checkin to branch.... (tinderbox still says closed
for verification)
Whiteboard: PDT+[SEEKING REVIEWS] → PDT+[ready to land]
Whiteboard: PDT+[ready to land] → PDT+ [ready to land]
Peter, pls check into the branch now!  Thanks.
Checked into both the trunk and the branch, marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Whiteboard: PDT+ [ready to land] → PDT+
verified fixed on mac commercial builds 2001-07-20-03-0.9.2 & 
2001-07-20-08-trunk
Status: RESOLVED → VERIFIED
I am wondering if this latest fix is causing bug 91617.
You need to log in before you can comment on or make changes to this bug.