Closed Bug 603679 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta8+
blocking1.9.2 --- -
status1.9.2 --- wontfix
blocking1.9.1 --- -
status1.9.1 --- wontfix

People

(Reporter: jimm, Assigned: Ehsan)

References

()

Details

Attachments

(2 files, 3 obsolete files)

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
blocking2.0: --- → ?
(ipc enabled or disabled)
Blocks: 545892
Better regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b36eeab1df8b&tochange=d45c87e58110

Which points us almost certainly to bug 286382.
Assignee: nobody → ehsan
Blocks: 286382
Investigating...
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.
(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.
(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...)
(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.)
(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!
blocking2.0: ? → beta7+
Can we back out 286382 until we find a resolution?
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?
blocking2.0: beta7+ → ?
Hooray!

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

That makes perfect sense.
Attached patch Patch (v1) (obsolete) — Splinter Review
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...
Attachment #483285 - Flags: review?(benjamin)
Flags: in-litmus?(marcia)
Comment on attachment 483285 [details] [diff] [review]
Patch (v1)

Aw, really?
Attachment #483285 - Flags: review?(benjamin) → review+
Yes!
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/fd54372f6b83
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
This should block branches because bug 286382 does.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Attachment #483285 - Flags: approval1.9.2.12?
Attachment #483285 - Flags: approval1.9.1.15?
blocking1.9.1: ? → .15+
blocking1.9.2: ? → .12+
Attachment #483285 - Flags: approval1.9.2.12?
Attachment #483285 - Flags: approval1.9.2.12+
Attachment #483285 - Flags: approval1.9.1.15?
Attachment #483285 - Flags: approval1.9.1.15+
This doesn't appear to be fixed. I'll test tomorrow's nightly to be certain.

http://www.miniclip.com/games/ice-racer/en/
Not fixed.
> #	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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(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.
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...
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?
Whiteboard: [waiting on bsmedberg]
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.
(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?  :(
I can't think of any way for us to fix this on branches without a shim or hook.
(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...
https://litmus.mozilla.org/show_test.cgi?id=13682 added to Litmus.
Flags: in-litmus?(marcia) → in-litmus+
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
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...
Maybe its the path that you start Firefox from?
Attachment #483285 - Flags: approval1.9.2.12-
Attachment #483285 - Flags: approval1.9.2.12+
Attachment #483285 - Flags: approval1.9.1.15-
Attachment #483285 - Flags: approval1.9.1.15+
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.
blocking1.9.1: .15+ → needed
blocking1.9.2: .12+ → needed
(In reply to comment #36)
> Maybe its the path that you start Firefox from?

No, that path has no significance, unfortunately...
Benjamin, what do you think about putting SetDllDirectory guards around all of our calls into plugins?
I think that's not likely to perform well or be feasible.
(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.
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.
(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.
Why can't Shockwave be OOP on the 3.6 branch?
(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.
It hasn't gotten a lot of testing.
(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?
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 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?
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
(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.
(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.
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
This patch makes sure that we don't use the SetDllDirectory protection for the shockwave director plugin.
Attachment #483285 - Attachment is obsolete: true
Attachment #486745 - Flags: review?(benjamin)
Whiteboard: [waiting on bsmedberg] → [has patch][needs review bsmedberg]
(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.
Status: REOPENED → ASSIGNED
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.
(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.
(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.
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 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?
Attachment #486745 - Flags: review?(benjamin) → review-
This patch adds a wrapper for SetDllDirectory stuff in toolkit/xre.
Attachment #486745 - Attachment is obsolete: true
Attachment #487435 - Flags: review?(benjamin)
Duplicate of this bug: 609607
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.
Attachment #487435 - Flags: review?(benjamin) → review+
Attached patch For check-inSplinter Review
With comments addressed.
Attachment #487435 - Attachment is obsolete: true
Whiteboard: [has patch][needs review bsmedberg] → [needs landing]
Shockwave 11 is still not working in the latest nightly build.
(In reply to comment #65)
> Shockwave 11 is still not working in the latest nightly build.

Fix hasn't landed yet.
(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.
(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.
http://hg.mozilla.org/mozilla-central/rev/413f719c1d73
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Shockwave still not working in today's nightly build http://hg.mozilla.org/mozilla-central/rev/4ef3abd2012c or the build mentioned in Comment 69
Attached file 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.
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).
(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.
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?
(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.
(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?
(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 ?
(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?
(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?
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)
(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.
It is true that this bug still exists if OOPP is disabled.  Unfortunately there are no obvious solutions for that case yet...
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 ?
(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).
Keywords: relnote
This patch is causing the Fennec (Mobile) Windows desktop builds to burn:
http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1289575500.1289584694.11832.gz
(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.
Depends on: 610556
Depends on: 612258
No longer depends on: 612258
(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?
(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?
"Win32 Fennec Desktop mozilla-central nightly" on http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mobile

I pushed a fix in bug 612167
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.
blocking1.9.1: needed → -
blocking1.9.2: needed → -
http://www.mozilla.com/en-US/firefox/4.0/releasenotes/

nothing in relnote.
this issue should be added to relnote.
The bug was already fixed. Non-OOPP configuration is not supported.
What should be written in relnote?
(In reply to comment #93)
> What should be written in relnote?

shockwave fails to load with OOPP disabled. (Windows)
We are not going to relnote that shockwave doesn't work in a non-default configuration which is not exposed in the UI.
Keywords: relnote
(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.
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.
You need to log in before you can comment on or make changes to this bug.