603679 - Shockwave fails to load (10-9 nightly and newer)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jim Mathies [:jimm]]

The 10-8 nightly works fine, in 10-9 the instance fails in NPP_New:

http://mxr.mozilla.org/mozilla-central/source/modules/plugin/base/src/nsNPAPIPluginInstance.cpp#419

The return error is NPERR_MODULE_LOAD_FAILED_ERROR.

Regression range doesn't have anything obvious in it.

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=10%2F8%2F2010&enddate=10%2F9%2F2010

Comment 1

[Jim Mathies [:jimm]]

(ipc enabled or disabled)

Comment 2

[Benjamin Smedberg]

Better regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b36eeab1df8b&tochange=d45c87e58110

Which points us almost certainly to bug 286382.

Comment 3

[(no longer active)]

Investigating...

Comment 4

[(no longer active)]

OK, I verified that plugin-container can load the shockwave DLL successfully.  There's a slight chance that this is actually not caused by bug 286382, I'm bisecting the range to see what changeset is the real offender.  I'll update this bug tomorrow.

Comment 5

[Jim Mathies [:jimm]]

(In reply to comment #4)
> OK, I verified that plugin-container can load the shockwave DLL successfully. 
> There's a slight chance that this is actually not caused by bug 286382, I'm
> bisecting the range to see what changeset is the real offender.  I'll update
> this bug tomorrow.

FWIW, I backed out that patch and the problem went away. It might be a dependent dll that shockwave is looking for which isn't in the dll search path.

Comment 6

[(no longer active)]

(In reply to comment #5)
> (In reply to comment #4)
> > OK, I verified that plugin-container can load the shockwave DLL successfully. 
> > There's a slight chance that this is actually not caused by bug 286382, I'm
> > bisecting the range to see what changeset is the real offender.  I'll update
> > this bug tomorrow.
> 
> FWIW, I backed out that patch and the problem went away.

Oh OK.  This is convincing enough!  :-)

> It might be a
> dependent dll that shockwave is looking for which isn't in the dll search path.

Yes, but I managed to trap firefox.exe and plugin-container.exe under the debugger, after the DLL was loaded in the latter, and also made sure that both processes are doing some IPC successfully...  Unfortunately I don't know a lot about the IPC code, so I didn't know where I should break in order to catch possible problems under the debugger...  I also set a breakpoint on patched_LdrLoadDll, but it didn't seem to get hit for any interesting DLL loads after the shockwave DLL was loaded (in fact, it was only called to load advapi32.dll from within Windows code, which succeeds...)

Comment 7

[Jim Mathies [:jimm]]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > OK, I verified that plugin-container can load the shockwave DLL successfully. 
> > > There's a slight chance that this is actually not caused by bug 286382, I'm
> > > bisecting the range to see what changeset is the real offender.  I'll update
> > > this bug tomorrow.
> > 
> > FWIW, I backed out that patch and the problem went away.
> 
> Oh OK.  This is convincing enough!  :-)
> 
> > It might be a
> > dependent dll that shockwave is looking for which isn't in the dll search path.
> 
> Yes, but I managed to trap firefox.exe and plugin-container.exe under the
> debugger, after the DLL was loaded in the latter, and also made sure that both
> processes are doing some IPC successfully...  Unfortunately I don't know a lot
> about the IPC code, so I didn't know where I should break in order to catch
> possible problems under the debugger...  I also set a breakpoint on
> patched_LdrLoadDll, but it didn't seem to get hit for any interesting DLL loads
> after the shockwave DLL was loaded (in fact, it was only called to load
> advapi32.dll from within Windows code, which succeeds...)

You can test this with ipc disabled, both modes fail.

Using api monitor, I see the base plugin (np32dsw.dll) calling LoadLibrary on "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll". This fails with the patch applied. The plugin makes the following calls:

np32dsw.dll	SetCurrentDirectoryW ( "C:\Windows\system32\Adobe\Shockwave 11" )	
np32dsw.dll	LoadLibraryW ( "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll" ) (failed)
np32dsw.dll	SetCurrentDirectoryW ( "C:\Program Files (x86)\Minefield" )

If I add back the following block of code in modules/plugin/base/src/nsPluginsDirWin.cpp, it loads:

-  typedef BOOL
-  (WINAPI *pfnSetDllDirectory) (LPCWSTR);
-  pfnSetDllDirectory setDllDirectory =
-    reinterpret_cast<pfnSetDllDirectory>
-    (GetProcAddress(GetModuleHandleW(L"kernel32.dll"), "SetDllDirectoryW"));
-  if (setDllDirectory) {
-    setDllDirectory(NULL);
-  }

This call was resetting the default search paths for dlls and adding the current working directory to the list.

I'm not cc'd into bug 286382, so I don't understand why this was removed. But the simple fix for this bug is to put back the call to setDllDirectory(NULL) here.

(http://www.rohitab.com/ has an alpha out of their api monitor 2.0 which was useful in looking at this.)

Comment 8

[(no longer active)]

(In reply to comment #7)
> You can test this with ipc disabled, both modes fail.

Oh, that's so much easier.  For some reason I had totally missed comment 1.  I'm building with IPC disabled right now.

> Using api monitor, I see the base plugin (np32dsw.dll) calling LoadLibrary on
> "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll". This fails with the patch
> applied. The plugin makes the following calls:
> 
> np32dsw.dll    SetCurrentDirectoryW ( "C:\Windows\system32\Adobe\Shockwave 11"
> )    
> np32dsw.dll    LoadLibraryW ( "C:\Windows\system32\Adobe\Shockwave
> 11\Plugin.dll" ) (failed)
> np32dsw.dll    SetCurrentDirectoryW ( "C:\Program Files (x86)\Minefield" )

OK, now the failure makes sense.

> If I add back the following block of code in
> modules/plugin/base/src/nsPluginsDirWin.cpp, it loads:
> 
> -  typedef BOOL
> -  (WINAPI *pfnSetDllDirectory) (LPCWSTR);
> -  pfnSetDllDirectory setDllDirectory =
> -    reinterpret_cast<pfnSetDllDirectory>
> -    (GetProcAddress(GetModuleHandleW(L"kernel32.dll"), "SetDllDirectoryW"));
> -  if (setDllDirectory) {
> -    setDllDirectory(NULL);
> -  }
> 
> This call was resetting the default search paths for dlls and adding the
> current working directory to the list.
> 
> I'm not cc'd into bug 286382, so I don't understand why this was removed. But
> the simple fix for this bug is to put back the call to setDllDirectory(NULL)
> here.

I just CCed you on that bug.  As you can see there, that is really not an option at all.

> (http://www.rohitab.com/ has an alpha out of their api monitor 2.0 which was
> useful in looking at this.)

Thanks for introducing this tool.  It's really useful!

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Can we back out 286382 until we find a resolution?

Comment 12

[(no longer active)]

There's no way that this is going to block beta7, as bug 286382 landed after the GECKO20b7pre_20101006_RELBRANCH branch was created, and it has not landed on that branch, so this is not a regression in beta7.

(In reply to comment #11)
> Can we back out 286382 until we find a resolution?

Given that this doesn't affect beta7, do you still suggest that we do this?

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Hooray!

I suggest it blocks beta8, be that via backout or fix.

Comment 14

[(no longer active)]

(In reply to comment #13)
> Hooray!
> 
> I suggest it blocks beta8, be that via backout or fix.

That makes perfect sense.

Comment 15

[(no longer active)]

Attachment: Patch (v1)

Apparently SetDllDirectory(NULL) only takes effect if the current directory is set after it.  This patch only relocates the SetDllDirectory call to make sure that it takes effect...

Comment 16

[Benjamin Smedberg]

Comment on attachment 483285 [details] [diff] [review]
Patch (v1)

Aw, really?

Comment 17

[(no longer active)]

Yes!

Comment 18

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/fd54372f6b83

Comment 19

[(no longer active)]

This should block branches because bug 286382 does.

Comment 20

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/44ca54433b56

Comment 21

[Jim Mathies [:jimm]]

This doesn't appear to be fixed. I'll test tomorrow's nightly to be certain.

http://www.miniclip.com/games/ice-racer/en/

Comment 22

[JK]

Not fixed.

Comment 23

[Masatoshi Kimura [:emk]]

> #	TID	Module	API	Return	Error
> 2	488	xul.dll	SetDllDirectoryW ( NULL )	TRUE	
> 3	488	xul.dll	SetCurrentDirectoryW ( "C:\Windows\system32\Adobe\Director" )	TRUE	
> 4	488	nspr4.dll	LoadLibraryExW ( "C:\Windows\system32\Adobe\Director\np32dsw.dll", NULL, 0 )	0x6fad0000	
> 5	488	xul.dll	SetCurrentDirectoryW ( "C:\ProgramData\Mozilla Firefox" )	TRUE	
> 6	488	xul.dll	SetDllDirectoryW ( "" )	TRUE	
> 7	488	VERSION.dll	LoadLibraryExW ( "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll", NULL, LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE )	0x6fa60002	
> 8	488	VERSION.dll	LoadLibraryExW ( "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll", NULL, LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE )	0x6fa60002	
> 9	488	np32dsw.dll	SetCurrentDirectoryW ( "C:\Windows\system32\Adobe\Shockwave 11" )	TRUE	
> 10	488	np32dsw.dll	LoadLibraryW ( "C:\Windows\system32\Adobe\Shockwave 11\Plugin.dll" )	NULL	
> 11	488	np32dsw.dll	SetCurrentDirectoryW ( "C:\ProgramData\Mozilla Firefox" )	TRUE	

Np32dsw.dll trys to load Plugin.dll AFTER dll directory is set back to "".
We need to shim Shockwave Player or complain to Adobe.

Comment 24

[Jim Mathies [:jimm]]

(In reply to comment #23)
> 
> Np32dsw.dll trys to load Plugin.dll AFTER dll directory is set back to "".
> We need to shim Shockwave Player or complain to Adobe.

Shimming a single player would fix it, but I have to wonder how many less known 3rd party plugins bug 286382 might have broken.

Comment 25

[(no longer active)]

OK, when I launch Firefox from the command line, we can load the Shockwave plugin without any problems.  When I launch it from Explorer, we can't...

Comment 26

[(no longer active)]

So, it seems that Shockwave is using SetCurrentDirectory in its NP_Initialize code to load Plugin.dll.  Calling SetDllDirectory like we do will obviously break that.  I see a couple of options here:

1. Contact Adobe and ask them to fix things.  I don't think this is practical.
2. Stop protecting against DLL loading attacks in plugin processes.  I don't particularly like this approach either, but I don't see how we can fix things otherwise.

Benjamin, what do you think?

Comment 27

[Benjamin Smedberg]

Can we call SetDllDirectory(pluginDirectory) and leave it set permanently for the plugin process? That won't help on branches (shockwave isn't OOPP on branch), but it will help on trunk.

Comment 28

[(no longer active)]

(In reply to comment #27)
> Can we call SetDllDirectory(pluginDirectory) and leave it set permanently for
> the plugin process?

No, because the plugin might be trying to load the DLL from another directory (which is the case at least with the Shockwave plugin).

> That won't help on branches (shockwave isn't OOPP on
> branch), but it will help on trunk.

Oh, so that means that my proposal in comment 26 isn't good enough for branches, right?  :(

Comment 29

[Benjamin Smedberg]

I can't think of any way for us to fix this on branches without a shim or hook.

Comment 30

[(no longer active)]

(In reply to comment #29)
> I can't think of any way for us to fix this on branches without a shim or hook.

But what would the said shim do?  I thought of hooking SetCurrentDirectoryW, saving it, and then trying to use it if the desired DLL exists in that directory in out LdrLoadDll hook, but that would defeat the whole purpose of this protection...

Comment 32

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=13682 added to Litmus.

Comment 34

[(no longer active)]

I backed out the patch form 1.9.2 for now until it's ready:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/71ef6d4ad1d3

Comment 35

[(no longer active)]

I have some new information, but I'm not yet sure how helpful this is.

Firefox is able to load Shockwave using these steps:

1. Run cmd.exe.
2. Run C:\mozilla-build\msys\bin\bash.exe --login
3. Run Firefox from within bash.

The key here seems to be running Firefox from bash when it's a login shell.  For whatever reason, the plugin cannot be loaded when Firefox is run from a non-login shell.  I've been reading the bash source code for most of today, and I haven't been able to track down what's the differentiating factor yet...

Comment 36

[Timothy Nikkel (:tnikkel)]

Maybe its the path that you start Firefox from?

Comment 37

[LegNeato]

I think we will pass on this for branches until it has baked/shipped in a FF4.0 beta. Rescinding approvals and setting blocking flags appropriately.

Comment 38

[(no longer active)]

(In reply to comment #36)
> Maybe its the path that you start Firefox from?

No, that path has no significance, unfortunately...

Comment 39

[(no longer active)]

Benjamin, what do you think about putting SetDllDirectory guards around all of our calls into plugins?

Comment 40

[Benjamin Smedberg]

I think that's not likely to perform well or be feasible.

Comment 41

[(no longer active)]

(In reply to comment #40)
> I think that's not likely to perform well or be feasible.

Except for that, the only workaround that I can think of is to revert the fix landed in bug 286382 and start to use the original approach in that bug, that is, passing absolute paths to LoadLibrary and similar functions.

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why don't we just sniff Shockwave and not call SetDllDirectory for it? Not many users have Shockwave installed I guess.

In parallel we should tell Adobe to fix their stuff. We could make the Shockwave-sniff not apply to new versions of Shockwave.

Comment 43

[(no longer active)]

(In reply to comment #42)
> Why don't we just sniff Shockwave and not call SetDllDirectory for it? Not many
> users have Shockwave installed I guess.
> 
> In parallel we should tell Adobe to fix their stuff. We could make the
> Shockwave-sniff not apply to new versions of Shockwave.

That won't solve the problem on the branches, because Shockwave is not out of process on branch, and is therefore affected by our global SetDllDirectory call.

Comment 44

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why can't Shockwave be OOP on the 3.6 branch?

Comment 45

[(no longer active)]

(In reply to comment #44)
> Why can't Shockwave be OOP on the 3.6 branch?

This is a question that bsmedberg can answer best, I think.

Comment 46

[Benjamin Smedberg]

It hasn't gotten a lot of testing.

Comment 47

[(no longer active)]

(In reply to comment #46)
> It hasn't gotten a lot of testing.

Is this something that we _can_ do?  If yes, what amount of testing would it need before we can mark it as safe to be run out of process on branch?

Comment 48

[Benjamin Smedberg]

Ask the QA group? Because shockwave isn't used nearly as much as Flash/Silverlight, it's a lot harder to get widespread QA on it.

Comment 49

[Bill Gianopoulos [:WG9s]]

It is so much not used that I wonder if we are just beating a dead horse trying to keep it working.  Isn't this an only supported on Windows thing?

Comment 50

[jcspence74]

Shockwave is supported on Windows and Mac.

Here are some usage numbers..
Available Sites for Shockwave Flash Embed
Standard Sites	161,711
	Sites within the top one million on the internet.
Extended Sites	1,222,785
	Sites within the additional ten million domains we crawl.
http://trends.builtwith.com/websitelist/Shockwave-Flash-Embed

If no longer supported.. it should be removed from Plugins:: Add-ons for Firefox
https://addons.mozilla.org/en-US/firefox/browse/type:7

Comment 51

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #50)
> Here are some usage numbers..
> Available Sites for Shockwave Flash Embed
> Standard Sites    161,711
>     Sites within the top one million on the internet.
> Extended Sites    1,222,785
>     Sites within the additional ten million domains we crawl.
> http://trends.builtwith.com/websitelist/Shockwave-Flash-Embed

Those stats are for Flash, not for the Adobe Shockwave Player that we're talking about here. That site doesn't even have stats for the Adobe Shockwave Player.

(In reply to comment #48)
> Ask the QA group? Because shockwave isn't used nearly as much as
> Flash/Silverlight, it's a lot harder to get widespread QA on it.

It also matters proportionally less.

Let's just make it OOP on the branch and move on. If that turns out to be a disaster, we can fix it.

Comment 52

[(no longer active)]

(In reply to comment #42)
> In parallel we should tell Adobe to fix their stuff. We could make the
> Shockwave-sniff not apply to new versions of Shockwave.

Filed bug 607832 about that.

Comment 53

[jcspence74]

Sorry.. got confused because they called it Shockwave Flash.
I did some more research on Shockwave.  It is mainly used for online gaming.
Seems to still be quit a few online gaming sites that use Shockwave.
http://www.google.com/search?q=online+shockwave+game+sites

Comment 54

[(no longer active)]

Attachment: Blacklist shockwave for SetDllDirectory

This patch makes sure that we don't use the SetDllDirectory protection for the shockwave director plugin.

Comment 55

[(no longer active)]

(In reply to comment #54)
> Created attachment 486745 [details] [diff] [review]
> Blacklist shockwave for SetDllDirectory
> 
> This patch makes sure that we don't use the SetDllDirectory protection for the
> shockwave director plugin.

It also includes a backout of the patch landed on m-c previously for this bug, as it's not effective.

Comment 56

[Masatoshi Kimura [:emk]]

Comment on attachment 486745 [details] [diff] [review]
Blacklist shockwave for SetDllDirectory

> +  typedef BOOL
> +  (WINAPI *pfnSetDllDirectory) (LPCWSTR);
> +  pfnSetDllDirectory setDllDirectory =
> +    reinterpret_cast<pfnSetDllDirectory>
> +    (GetProcAddress(GetModuleHandleW(L"kernel32.dll"), "SetDllDirectoryW"));
> +  if (protectCurrentDirectory && setDllDirectory) {
> +    setDllDirectory(NULL);
> +  }
> +
>    nsresult rv = plugin->Load(outLibrary);
>    if (NS_FAILED(rv))
>        *outLibrary = NULL;
>  
> +  if (protectCurrentDirectory && setDllDirectory) {
> +    setDllDirectory(L"");
> +  }
If protectCurrentDirectory is false (i.e. for Shockwave Player), dll directory will not reset to NULL on plugin loading. The effect is the opposite.
You can't resolve the issue for non-OOPP case as you said in comment #43.

Comment 57

[(no longer active)]

(In reply to comment #56)
> If protectCurrentDirectory is false (i.e. for Shockwave Player), dll directory
> will not reset to NULL on plugin loading. The effect is the opposite.

I'm not sure what you mean.  For shockwave player, we never call SetDllDirectory("") in the first place, so why is not calling it with NULL a problem?

> You can't resolve the issue for non-OOPP case as you said in comment #43.

Yes, that's true.

Comment 58

[Masatoshi Kimura [:emk]]

(In reply to comment #57)
> I'm not sure what you mean.  For shockwave player, we never call
> SetDllDirectory("") in the first place,
nsBrowserApp.cpp includes nsWindowsWMain.cpp without defining XRE_DONT_PROTECT_DLL_LOAD.
http://mxr.mozilla.org/mozilla-central/source/browser/app/nsBrowserApp.cpp#60
So SetDllDirectory("") will be called on app startup for non-OOPP case. Am I missing something?
> so why is not calling it with NULL a problem?
It may not make things worse, (Shockwave Player will not work anyway) but protectCurrentDirectory check is unneeded.

Comment 59

[(no longer active)]

As I've stated in this bug before, this patch does not address the non-OPP case, and I currently do not have a good solution at hand to address this problem.  Let's keep the discussion relevant to what the patch tries to address.

Comment 60

[Benjamin Smedberg]

Comment on attachment 486745 [details] [diff] [review]
Blacklist shockwave for SetDllDirectory

We now have the SetDllDirectory code copied into at least four different files. Can we make a helper function in some header and include it from these different places in order to stay sane?

Comment 61

[(no longer active)]

Attachment: Blacklist shockwave for SetDllDirectory

This patch adds a wrapper for SetDllDirectory stuff in toolkit/xre.

Comment 63

[Benjamin Smedberg]

Comment on attachment 487435 [details] [diff] [review]
Blacklist shockwave for SetDllDirectory

Please put NS_SetDllDirectory in the mozilla namespace and make it nonstatic.
Please remove the two trailing underscores from the #ifndef nsSetDllDirectory_h

r=me with those changes.

Comment 64

[(no longer active)]

Attachment: For check-in

With comments addressed.

Comment 65

[Chris B.]

Shockwave 11 is still not working in the latest nightly build.

Comment 66

[Gary [:streetwolf52]]

(In reply to comment #65)
> Shockwave 11 is still not working in the latest nightly build.

Fix hasn't landed yet.

Comment 67

[Chris B.]

(In reply to comment #66)
> (In reply to comment #65)
> > Shockwave 11 is still not working in the latest nightly build.
> 
> Fix hasn't landed yet.

Understandable - I just wasn't sure if attachment 488708 [details] [diff] [review] fixed the problem or not.  Is this a Firefox coding issue or an Adobe issue (where Shockwave 11 just doesn't recognize Firefox 4 yet, because Firefox 4 is still in beta form)?  Kind of what happened with software during the transition from Vista to Windows 7.

Comment 68

[(no longer active)]

(In reply to comment #67)
> (In reply to comment #66)
> > (In reply to comment #65)
> > > Shockwave 11 is still not working in the latest nightly build.
> > 
> > Fix hasn't landed yet.
> 
> Understandable - I just wasn't sure if attachment 488708 [details] [diff] [review] fixed the problem or
> not.  Is this a Firefox coding issue or an Adobe issue (where Shockwave 11 just
> doesn't recognize Firefox 4 yet, because Firefox 4 is still in beta form)? 
> Kind of what happened with software during the transition from Vista to Windows
> 7.

This is caused by a bad coding practice in the Shockwave plugin, and we're trying to work around it so that the plugin can be loaded.  Please retest with a nightly version produced after this bug is marked as RESOLVED FIXED.

Comment 69

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/413f719c1d73

Comment 70

[rob64rock]

Shockwave still not working in today's nightly build http://hg.mozilla.org/mozilla-central/rev/4ef3abd2012c or the build mentioned in Comment 69

Comment 71

[Jim Mathies [:jimm]]

Attachment: simple test case

Not working here either in a simple test case. This should load up shockwave and display a message telling you to visit a site to play the game.

Comment 72

[jcspence74]

http://www.adobe.com/shockwave/welcome/
http://www.miniclip.com/games/ice-racer/en/
https://bug603679.bugzilla.mozilla.org/attachment.cgi?id=489209

These all work for me.  Maybe you not have the latest version of Shockwave installed?

Comment 73

[Chris B.]

I agree with comment 72 - those sites all work for me as well with Shockwave 11.5.9.615.  The issue with the bug was that Minefield / Firefox 4 Beta 8 (Nightlies) were not recognizing Shockwave 11 - Shockwave TEN was working just fine.

I am using Minefield version 11/9/10 08:47 PST. 

For comment 71, make sure you have BOTH:
  1) Shockwave 11.5.9.615
  2) Minefield Nightly from 11/9/10

This was still broken in last night's build (11/8/10), so maybe that is what you still have installed for comment 69 and comment 71.  Also, the 11/9/10 Nightly took longer than normal to post (usually posts by 9:00 AM EST, but did not see until about 12:00 PM EST).

Comment 74

[Bill Gianopoulos [:WG9s]]

(In reply to comment #71)
> Created attachment 489209 [details]
> simple test case
> 
> Not working here either in a simple test case. This should load up shockwave
> and display a message telling you to visit a site to play the game.

This works fine for me.  Are you sure you have ipc plugins enabled?  As stated in earlier comments, this still does not work in the non-OPP case.

Comment 75

[(no longer active)]

All of the test cases here work for me using Shockwave 11.5.8.612.  If someone is seeing problems with the latest nightly, can you please post your about:support contents?

Comment 76

[rob64rock]

(In reply to comment #70)
> Shockwave still not working in today's nightly build
> http://hg.mozilla.org/mozilla-central/rev/4ef3abd2012c or the build mentioned
> in Comment 69

Sorry I didn't read Comment 59 , after enabling OOPP it Shockwave Plugin started working.

Comment 77

[(no longer active)]

(In reply to comment #76)
> (In reply to comment #70)
> > Shockwave still not working in today's nightly build
> > http://hg.mozilla.org/mozilla-central/rev/4ef3abd2012c or the build mentioned
> > in Comment 69
> 
> Sorry I didn't read Comment 59 , after enabling OOPP it Shockwave Plugin
> started working.

How had you disabled OOPP in the first place?  And why?

Comment 78

[pal-moz]

(In reply to comment #77)
> (In reply to comment #76)
> > (In reply to comment #70)
> > > Shockwave still not working in today's nightly build
> > > http://hg.mozilla.org/mozilla-central/rev/4ef3abd2012c or the build mentioned
> > > in Comment 69
> > 
> > Sorry I didn't read Comment 59 , after enabling OOPP it Shockwave Plugin
> > started working.
> 
> How had you disabled OOPP in the first place?  And why?

set "dom.ipc.plugins.enabled" to false.


"Shockwave fails to load with OOPP disabled" is intended/expected ?

if not, should file a bug ?

Comment 79

[Chris B.]

(In reply to comment #74)
> (In reply to comment #71)
> > Created attachment 489209 [details] [details]
> > simple test case
> > 
> > Not working here either in a simple test case. This should load up shockwave
> > and display a message telling you to visit a site to play the game.
> 
> This works fine for me.  Are you sure you have ipc plugins enabled?  As stated
> in earlier comments, this still does not work in the non-OPP case.

What exactly is OPP and what does it have to do with Shockwave?

Comment 80

[Chris B.]

(In reply to comment #74)
> (In reply to comment #71)
> > Created attachment 489209 [details] [details]
> > simple test case
> > 
> > Not working here either in a simple test case. This should load up shockwave
> > and display a message telling you to visit a site to play the game.
> 
> This works fine for me.  Are you sure you have ipc plugins enabled?  As stated
> in earlier comments, this still does not work in the non-OPP case.

What exactly is OPP and what does it have to do with Shockwave?

Comment 81

[LegNeato]

OOPP or OPP is another name for plugin crash protection (http://blog.mozilla.com/blog/2010/06/22/firefox-3-6-4-with-crash-protection-now-available/). When OOPP is enabled, it means plugins run in their own process, and this bug doesn't show up anymore. This is the default case for FF4. With OOPP disabled (also called in process plugins), this bug is still an issue (which is default for <= 3.6.x)

Comment 82

[Jim Mathies [:jimm]]

(In reply to comment #71)
> Created attachment 489209 [details]
> simple test case
> 
> Not working here either in a simple test case. This should load up shockwave
> and display a message telling you to visit a site to play the game.

Confirmed, working. My update hadn't applied yet.

Comment 83

[(no longer active)]

It is true that this bug still exists if OOPP is disabled.  Unfortunately there are no obvious solutions for that case yet...

Comment 84

[pal-moz]

this bug should be added to Beta 7 Release note "Known Issues" ?


and
(In reply to comment #83)
> It is true that this bug still exists if OOPP is disabled.

no bug for this issue ?

Comment 85

[Chris B.]

(In reply to comment #84)
> this bug should be added to Beta 7 Release note "Known Issues" ?
> 
> 
> and
> (In reply to comment #83)
> > It is true that this bug still exists if OOPP is disabled.
> 
> no bug for this issue ?

YES - I think this bug (bug 603679) should be added to the Known Issues section on http://www.mozilla.com/en-US/firefox/all-beta.html because the build date on the official Beta 7 release is 11/4/10.  This Shockwave issue was fixed in the 11/9/10 build.  Maybe you can also put a note saying that the fix didn't land before the Beta 7 release but the issue is resolved and Shockwave 11 will work in Beta 8.  The reason for this is because when I look at "Known Issues" I look at it being bugs still being worked on (which 95% of the time it is).  Once Beta 8 is officially released, then you can remove this bug as a known issue on the above page.  A lot of people use Shockwave and if they know it will work in Beta 8 in the next couple of weeks more people will download Firefox 4 Beta 8.  They are not going to download a browser that breaks their favorite websites (Known Issues list).

Comment 86

[Mark Finkle (:mfinkle) (use needinfo?)]

This patch is causing the Fennec (Mobile) Windows desktop builds to burn:
http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1289575500.1289584694.11832.gz

Comment 87

[(no longer active)]

(In reply to comment #86)
> This patch is causing the Fennec (Mobile) Windows desktop builds to burn:
> http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1289575500.1289584694.11832.gz

This should be fixed in bug 610556.  The mobile tree seems to be green now.  Let me know if there are more issues to solve.

Comment 88

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #87)
> (In reply to comment #86)
> > This patch is causing the Fennec (Mobile) Windows desktop builds to burn:
> > http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1289575500.1289584694.11832.gz
> 
> This should be fixed in bug 610556.  The mobile tree seems to be green now. 
> Let me know if there are more issues to solve.

The Windows nightly is still burning. Does a patch need to land in the mobile-browser tree?

Comment 89

[(no longer active)]

(In reply to comment #88)
> (In reply to comment #87)
> > (In reply to comment #86)
> > > This patch is causing the Fennec (Mobile) Windows desktop builds to burn:
> > > http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1289575500.1289584694.11832.gz
> > 
> > This should be fixed in bug 610556.  The mobile tree seems to be green now. 
> > Let me know if there are more issues to solve.
> 
> The Windows nightly is still burning. Does a patch need to land in the
> mobile-browser tree?

I don't see that in <http://tbpl.mozilla.org/?tree=Mobile>.  Can you please point me to a sample log?

Comment 90

[Mark Finkle (:mfinkle) (use needinfo?)]

"Win32 Fennec Desktop mozilla-central nightly" on http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mobile

I pushed a fix in bug 612167

Comment 91

[LegNeato]

We're not going to take this on branch because we are not going to take the bug that causes this. It seems too risky, there is too much potentially to break a whole class of lesser-known plugins, the workarounds likely wouldn't even work in 3.5, etc.

Comment 92

[pal-moz]

http://www.mozilla.com/en-US/firefox/4.0/releasenotes/

nothing in relnote.
this issue should be added to relnote.

Comment 93

[Masatoshi Kimura [:emk]]

The bug was already fixed. Non-OOPP configuration is not supported.
What should be written in relnote?

Comment 94

[pal-moz]

(In reply to comment #93)
> What should be written in relnote?

shockwave fails to load with OOPP disabled. (Windows)

Comment 95

[Benjamin Smedberg]

We are not going to relnote that shockwave doesn't work in a non-default configuration which is not exposed in the UI.

Comment 96

[pal-moz]

(In reply to comment #95)
> We are not going to relnote that shockwave doesn't work in a non-default
> configuration which is not exposed in the UI.

OK.
sorry for spamming.

Comment 97

[AndreiD[QA]]

WFM on:
Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre

It seems that the issue is not reproducible anymore. Schockwave clips are loading normally on a default FFx configuration.