Visit the page, notice the crash. NPSWF32 is the Flash plugin. I have attached a copy of the page just in case the HTML changes. Looking at the stack trace, looks like we are freeing memory that the plugin still needs. CC:ing Kevin. This may be your bug as it happens in the view manager.

Adding keywords.... Um...I guess the Javascript in the attached testcase is sniffing for Nav 4 and doesn't work. However, the attached testcase does still crash in Mozilla.

nfl pages have a topcrash in bug 55616 - is this related?

flash related nfl crash: bug 60895

raising bug severity to critical since I crash on other websites too with the same stack trace(powershot.com, grandhoteltijuana.com ). Build :1219 trunk on windows.

actually...BLOCKER

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

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

av writing from serge's box: Looks like we are hitting a major regression. It crashes on _any_ Flash enabled site now. Shrirang if you can confirm that please alter the summary. Meanwhile we need to investigate what the cause of the regression was. It crashes after ::SetWindowsPos call on the plugin window.

confirmed with today's trunk build that this crashes on any flash enabled page.Updated summary.

In the dup bug 63284, H-J says "you need to go back to build 2000121521 in order to get this baby run!" Also, looking in the debugger, looks like mWnd of nsWindow is pointing off to space. Andre, that probably matches the fact that you say we crash after ::SetWindowsPos. Are you already looking at this one, or would you like me to look at it? Oh, and if anyone is looking for a workaround to get pages with Flash to work, simply remove NPSWF32 from your Netscape 4.x and Mozilla/Netscape 6 plugin folders.

Okay, I found the problem. It is with widget/src/windows/nsWindow.cpp in version 3.07. Makoto Kato checked in a fix to bug 55256. Backing out his changes and going to version 3.06 does not crash. CC:ing him and Robert O'Callahan to possibly work out a fix that does not crash Flash pages

This is a problem with all Win32 plugins due to the way they subclass the plugin window that is created for them by the browser. The plugins are calling SetWindowLong thinking the created window is ANSI rather than UNICODE. Since the plugins are compiled non-UNICODE the call goes to SetWindowLongA. The default window proc that is returned is incorrect for ANSI compiles. Eventually a crash occurs when the plugin passes the call to what it thinks is the default window proc.

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

Oops! That was stupid. Someone should back out Makoto's checkin if they haven't already --- there is no easy fix. I am guilty :-(.

I think a Flash plug-in's bug. If not so, does anyone know the plug-in that AV occurs other than Flash??

This crashes the soon to be released xpcom Beatnik plugin and any Win32 plugin based on the 4x NPAPI sample. For example, http://lxr.mozilla.org/seamonkey/source/modules/plugin/test/npsimple.cpp#1098

Makoto --- Sean's analysis is correct. We cannot change the plugin's window from ANSI to Unicode without breaking things. Let's revert your changes for now.

1. this is not a reversal of the patch that causes the bug 2. the comments are incorrect - plugins are not causing this bug, they are exposing it. 3. this was not tested with the npsimple plugin 4. Is it good practice to mix ANSI and Unicode calls on an HWND? Makoto, can you tell us what internal HWND members/structures are getting clobbered by calling SetWindowLongA on a unicode HWND? To understand this question, consider what is happening right now. The plugin calls GetWindowLongA on a unicode HWND and gets a value like 0xffff1abc. When you call SetWindowLongA on a unicode HWND it will overwrite that value. What does that value represent?

Makoto, please revert your original patch. We are not going to try any more changes until that patch is reverted. This is not a Flash bug or a plugins bug, it is our bug and we are going to fix it the right way, by producing a correct fix for bug 55256.

To Robert Do you know how to create Windows NT/2000's GLOBALIZATION/M10L application??? The way of fixing bug 55256 only uses unicode window class. There is no way other than using unicode window class. Bacause WM_CHAR and WM_ IME_CHAR may return "?" charactor when current code page is different to keyboard code page. If Mozilla is not GLOBALIZATION/M10L application, bug 55256 should be mark as WONTFIX.

To sean The mix mode of Unicode and ANSI window class is possible. Because using Unicode window class, Windows does not convert WM_CHAR/WM_IME_CHAR. But using ANSI window class, Windows only converts WM_CHAR/WM_IME_CHAR using current code page by Windows self. So, bug 55256 occurs when current code page is different to keyboard code page. And I tested some Plug-in with new fix. But I do not happen the regression.

Mozilla's window procedure (widget/src/windows/nsWindows.cpp) support ANSI mode and Unicode mode. If you input hindi language via plug-in, some problem occurs. But old 4.x plug-in cannot use hindi language due to ANSI window class.

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

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

Asa: This should be a 0.7 stop-ship. The majority of commercial sites have some level of flash content on them, and we crash on all of them. There is a known fix for this bug -- namely backing out Makoto Kato's fix to bug 55256 in version 3.07 of widget/src/windows/nsWindow.cpp -- so we could get leaf to do that fix and make a build for you on an ultra-mini branch. Marking top100 (fox.com amongst others) and pre-emptively marking mostfreq (there are already 5 dupes after barely 10 days, and I have personally run into this crasher a good dozen times over the last week).

With Makoto Kato 12/24 patch, I no longer crash on any flash site. I also tried Shockwave, Acrobat, and Quicktime with no crash. Patch looks good. Andre/Robert what do you think?

1. The patch was not tested with the npsimple plugin. The patch does not address the crash that occurs with xpcom plugins - only 4x plugins. 2. The patch assumes internal knowledge of an opaque pointer - HWND. What internal HWND members/structures are overwritten by calling SetWindowLongA on an HWND that was created with a call to CreateWindowExW? To understand this question, consider why the crashes occur. The plugin calls GetWindowLongA on a unicode HWND and gets a value like 0xffff1abc. When you call SetWindowLongA on a unicode HWND it will overwrite that value. What does that value represent or how does it affect the operation of the window? Is your testing sufficient to guarantee that no plugins are affected by the change?

I agree with Sean. We need more testing/understanding before we can accept it.

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

Makoto's patch has to be backed out. I'd do it but I'm afraid of messing up the tree because a) I've never done a CVS backout before and b) I've just migrated to Linux and my system is not in a completely coherent state yet. After reading more MSDN and thinking about it some more, I think Makoto may actually be right. http://msdn.microsoft.com/library/psdk/winbase/unicode_21ta.htm But we still have to back out the current mess, agree on the correct fix, test it, and then check the whole thing in.

leaf successfully backed out the 3.307 revision of of widget/src/windows/nsWindow.cpp without breaking his local build. we were planning on doing that for the 0.7 branch.

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

Rob - thanks for digging up that msdn url. Looks like it addresses my main concern about calling SetWindowLongA on a unicode HWND. One thing to keep in mind is that plugins are probably not calling CallWindowProc (I know the Beatnik plugin doesn't - it was originally based on the old 4x NPAPI sample). Same for the npSimple xpcom plugin at: http://lxr.mozilla.org/seamonkey/source/modules/plugin/test/npsimple.cpp#1155 This *shouldn't* be an issue since the plugin windows as maintained by the browser *probably* won't be handling text params. The other problem about the patch still stands though - it does not address xpcom plugins.

That is why Makoto is right --- the plugins should all be using CallWindowProc, directly calling the proc is wrong. I don't understand the problem with XPCOM plugins, they should be using CallWindowProc too and it should just work. Given that people do directly call the window proc and we can't do anything about that, I think Makoto's proposed fix is OK. I'm still not comfortable with checking in "a fix on a fix", but if you all are comfortable with it, I guess I'd accede. I still think it would be better if we made plugin windows ANSI, though. That seems like a more robust solution. But then, I don't have a patch for that :-). > This *shouldn't* be an issue since the plugin windows as maintained by the > browser *probably* won't be handling text params. I don't know what you mean here. Once the plugin uses SetWindowLongA to install its window proc, Windows will convert all text parameters in messages to ANSI when it calls the plugin's window proc. There shouldn't be any problems.

>the plugins should all be using >CallWindowProc, >directly calling the proc is wrong. It only became 'wrong' with the introduction of unicode HWNDs in mozilla. There is nothing wrong with calling a window proc directly in and of itself. >I don't understand the problem with XPCOM >plugins, they should be using CallWindowProc too and it should just work. The problem with xpcom plugins isn't about CallWindowProc - it's that they implement nsIPluginInstance themselves. The proposed fix changes the implementation of the 4x plugin wrapper which implements nsIPluginInstance. XPCOM plugins do not get wrapped by ns4xPluginInstance - they directly implement nsIPluginInstance. >I still think it would be better if we made plugin windows ANSI, though. That >seems like a more robust solution. Agree. One alternative would be to add a member to nsWindow that flags the window as either unicode or ANSI. Then rather than relying on the static mIsNT toolkit member to determine flow, use that new nsWindow member. When windows are created for plugins, set the value so that ANSI calls are used regardless of OS. >I don't know what you mean here. Once the plugin uses SetWindowLongA to >install its window proc, Windows will convert all text parameters in messages >to ANSI when it calls the plugin's window proc. There shouldn't be any >problems. The point I was tring to make is that since the window procs are called directly rather than via CallWindowProc the text parameters will NOT get converted when the plugin calls the original mozilla window proc for unhandled messages.

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

http://msdn.microsoft.com/library/psdk/winui/winprocs_9dlv.htm > An application must pass any messages not processed by the new window > procedure to the previous window procedure by calling CallWindowProc. I think that's a pretty clear statement that directly calling the window proc, is, at best, deprecated. If your XPCOM plugin used CallWindowProc, I think it would work fine without any more patches. According to MSDN, GetWindowLongA/CallWindowProcA should work fine even on Unicode windows.

Yes Rob you are correct. Plugins should use CallWindowProc rather than calling the window proc directly. Apologies to Makoto. The bugs are in each individual plugin, not the browser. The patch should address this shortcoming of 4x plugins. Released XPCOM plugins are SOL and will cause post Dec 16 mozilla builds to crash.

I hope the plugin issue gets sorted out as soon as possible.

when do you think pages with flash content will be possible to display? tried it with build 2000122720 and it still crashes.

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

Sean, would you please elaborate on your statement 'Released XPCOM plugins are SOL and will cause post Dec 16 mozilla builds to crash'?

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

Since the attached workaround works only for 4x plugins, any xpcom plugin that has been publicly released and currently works in Netscape 6 will cause a crash in post Dec 16 builds of Mozilla if they call the window proc directly rather than via CallWindowProc (an unfortunate side-effect of modeling plugins on the NPAPI SDK sample plugin or npSimple XPCOM plugin). Unfortunately the only example of a released xpcom plugin that I'm aware of is the 2.1 Beatnik player (http://www.beatnik.com/software/player.html) - works in N6 and Mozilla builds up to Dec 16 but will cause the browser to crash on all win builds after Dec 16.

SOL --> Shit out of luck

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

I don't understand the 12/24 patch. mOrigProc is a Unicode window proc. SetWindowLongA should take an ANSI window proc. I don't understand why this patch works at all. I also don't understand why no-one wants to back the original patch out of the trunk. I'm still annoyed that Makoto didn't do it.

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

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

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

Who should back what out of the trunk when? I'm on staff@mozilla.org and we're wondering whether we need to hop in and "mediate", which would be unfortunate. av, can you take charge? /be

Robert, The replaced window proceduer is nsWindow::WindowProc(). This proceduer is supporting ANSI class and Unicode class. If most user uses the environment that WM_CHAR returns only 8bit char. So problem does not occur. But if a problem occur, the user uses Hindi or Almenia language etc in Plug-in. And old Netscpae 4.x cannot use Hindi or Almenia language in Plug-in.

Added self to CC.

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

> The replaced window proceduer is nsWindow::WindowProc(). This proceduer is > supporting ANSI class and Unicode class. I don't think it supports ANSI on NT --- it will always call DefWindowProcW, won't it?

The way I understand it, yes - nsWindow::WindowProc() will always call DefWindowProcW on NT. Which would probably be ok as long as plugin windows did not forward unhandled messages that take text params back to the mozilla proc. That's the point I was trying to make in the last paragraph of my 2000-12-28 14:16 post. When the plugin forwards unhandled messages to mozilla, any text params will be ANSI and will not be converted to unicode (since automatic conversion is supposed to be handled by the missing call to CallWindowProc).

The patch might work for the following reasons: It might that there are no unhandled text messages that get forwarded back to the Mozilla unicode proc. Or it might be that the Mozilla unicode proc is robust enough to handle ANSI text params in its unicode proc. Something to test with this patch is a 4x plugin that does the correct thing to begin with (uses CallWindowProc vs directly invoking the window proc) (maybe Real or Adobe got it right).

Makoto: people are currently blocked by the crash, from the other hand, we still do not have a concensus on how to proceed. Would you please reverse your changes that caused the original problem as soon as possible? Looks like we need some time to discuss the issue but we cannot afford to keep people from testing/developing any longer.

av, For release Mozilla 0.7, I agree for back to old code (changing nsToolkit.h) But When next tree is open, I re-check in same code for all fixes. OK? And change a few widget codes for Robert's comments.

Widget side fix mistakes one line. ::IsWindowUnicode() -> ::IsWindowUnicode(mWnd) And Plug-in side fix supports both old 4.x plug-in and new 6.x plug-in.

Makoto, please go ahead and revert your changes on the trunk. But you will need to get new reviews before you can check in again. I wouldn't have approved your original checkin if I had realized that it was going to break 4.x plugins.

ok. I agree for release Mozilla 0.7 But could you review new code??

Don't worry, we WILL get the original patch and some new code reviewed eventually :-). First of all, in your patch for nsWindow.cpp, around line 980, you'd want to "return ::DefWindowProcA" instead of just calling it and then calling nsToolkit::DefWindowProc right after that. I actually don't like the approach of making nsWindow::WindowProc try to guess whether it should be acting as an ANSI or Unicode. It's making nsWindow more complicated. Also, the guess will sometimes be wrong ... IsWindowUnicode() doesn't tell us whether nsWindow::WindowProc was called with Unicode parameters, it tells us whether the subclassed window proc was called with Unicode parameters. For example, if a plugin does the right thing by installing an ANSI window proc and then using CallWindowProc to call the original proc (WindowProc), then WindowProc will be called with Unicode parameters but IsWindowUnicode will return FALSE. Therefore your WindowProc patch will incorrectly assume that it has ANSI parameters. That is wrong. I can think of two clean solutions. One is to simply make sure that windows for plugins are created as ANSI windows in the beginning. This means that Hindi and Armenian and these other alternative codepages won't work in any plugins, but they didn't work before anyway. The other clean solution is to modify ns4xPluginInstance, similar to what you had before, but instead of calling SetWindowProcA(..., mOrgProc), install a wrapper function, using SetWindowProcA(..., ANSIWrapperProc), where LONG ANSIWrapperProc(HWND wnd, ...) { ns4xPluginInstance p = // obtain ns4xPluginInstance pointer from wnd return CallWindowProc(p->mOrgProc, ...); } This solution means that the parameters will automatically be converted to Unicode, so we don't have to worry about that in nsWindow. In a sense, we're just catching the incorrect direct calls to the window proc and converting them to correct calls to CallWindowProc. We could do this in nsObjectFrame, but I think that it's better to do this only for ns4x plugins, and deliberately break XPCOM plugins. There are very few of the latter and we might as well force them to get things right now. Certainly it's not very hard for XPCOM plugin authors to fix their code, and the changed code should work fine with NS6.0. And hey, they might want to use alternative keyboard code pages in their plugins :-).

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

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

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

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

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

Is this going to be backed-out or patched anytime soon?

It is reviewer process. But all member does not response. Can anyone review/super-review??

m_kato: you need to pick a reviewer by area, per the document at http://www.mozilla.org/hacking/reviewers.html -- just mailing to reviewers@mozilla.org won't get anywhere, because no individual can be held accountable. If you have asked a particular super-reviewer for review and received no response, please mail me and that reviewer, and I'll try to find a substitute. The short answer is that roc+moz@cs.cmu.edu can super-review this, av can give it module owner review, and we're done. Everyone I just named agree? /be

We should be just backing out the previous checkin, not adding all these ifdefs and other stuff. In particular, the first three changes to nsWindow do not seem to be relevant. I cannot approve. PS, sorry for the delay, I'm on the road.

Robert, can you review/super-review?? I will be able not to check in until tomorrow since I am in Chicago airport:-<

sorry, the following is mistake merge. *** 2916,2921 **** --- 2879,2885 ---- mIsAltDown = IS_VK_DOWN(NS_VK_ALT); } else { // WM_KEYUP // If the Context Code bit is down and we got a WM_KEY + // If the Context Code bit is down and we got a WM_KEY // it is a key press for character, not accelerator // see p246 of Programming Windows 95 [Charles Petzold] for details mIsControlDown = (0 == (KF_ALTDOWN & HIWORD(lParam)))&& IS_VK_DOWN(NS_VK_CONTROL); *************** *** 2952,2957 **** --- 2916,2922 ---- mIsAltDown = IS_VK_DOWN(NS_VK_ALT); } else { // WM_KEYUP // If the Context Code bit is down and we got a WM_KEY + // If the Context Code bit is down and we got a WM_KEY // it is a key press for character, not accelerator // see p246 of Programming Windows 95 [Charles Petzold] for details mIsControlDown = (0 == (KF_ALTDOWN & HIWORD(lParam)))&& IS_VK_DOWN(NS_VK_CONTROL);

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

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

Can you use diff -u everywhere, please? It's easier to read. The patch looks good except for one thing --- why are you changing the NS_IMM_GETCONVERSIONSTATUS, NS_IMM_NOTIFYIME and NS_IMM_SETCONVERSIONSTATUS macros?

Sean, whatever happens, that fix to nsSimple should go in ASAP. I'm not really the appropriate super-reviewer but it's so simple, and it's Windows code, so I'll give it an sr=roc+moz.

robert, I only back to original codes since you say "We should be just backing out the previous checkin, not adding all these ifdefs and other stuff" See Bug 55256 and http://bugzilla.mozilla.org/showattachment.cgi?attach_id=20679 Do you agree this macro keeps bug 55256's fix?

This bug is open for three weeks now, it is a major blocker and regression, it causes a crash on a lot of sites, and several of us (including me) have run into this more than once. I'm wondering how difficult it is to just back out those changes in version 3.07 of widget/src/windows/nsWindow.cpp? Then we would have a working browser again and we could try to find a fix for bug 55256 without hurry. Thanks!

st.n, back out code is http://bugzilla.mozilla.org/showattachment.cgi?attach_id=22021 I cannot check in until Robert apporove.

The problem here may be that people are using the term "backing out" for different things: - "Backing out the change" can mean applying a CVS command that reverses the change, and committing the result as-is. If this fixes the blocker and does not introduce severe regressions, this is usually a safe, simple and fast solution, and can be done without going through a complete review process; usually it's just a matter of approval. If such a method could be applied here, this would be best, and Robert has said many times that he would approve such a solution. An example for such a set of commands can be obtained by clicking on the "Show commands which could be used to back out these changes" link on a bonsai page like this: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=mozilla%2Fwidget%2Fsrc%2Fwindows&file=nsWindow.cpp&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=12%2F16+00%3A00&maxdate=12%2F16+23%3A59&cvsroot=%2Fcvsroot In this case the result is very simple: cvs update -j3.307 -j3.306 mozilla/widget/src/windows/nsWindow.cpp - If for whatever reason such a solution cannot be applied, as Makoto seems to argue, then things get complicated, because a "backout patch" has to be created, reviewed, super-reviewed etc., including all the "red tape" associated with the process. In this case, all the diffs should be created by "diff -wu" to make them more readable. Attachment 22021 [details] [diff] seems to be a "diff -c". Makoto, can you do add the other format, please?

OK, I see that the macros you're changing aren't actually being used yet (#ifdef MOZ_AIMM). I approve the patch. (You have compiled and tested it, right?) sr=roc+moz I don't care whether we apply this patch or do the CVS thing. I think either way will work. We just need to get it done.

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

I guess as a module owner I should give an "a=" to Sean's patch for the Simple plugin. a=av.

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

check in old code. But this bug keeps open for bug 55256. And av@netscape.com, please assin this bug to me

reassigning

Just verified that flash is working fine on today's trunk builds (0110) on all platforms. I get no crashes.

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

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

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

Crasher for a "top 2" plug-in (Flash or RealPlayer). Nominating nsbeta1.

This should be fixed since Jan 11. Sean has also checked in his patch to nsSimple. I'm marking this FIXED.

Just to verify that this bug was fixed, I've been trying to go through all of the pages listed as dup's, and visit any URLs that are listed there. So far, every time I've done this, Mozilla has crashed before I get through all of them. This is on TalkBack Build 2001011208, on Linux, with no plugins at all installed. Each time I'm asked to install the shockwave or Java plugin by these pages, I click on cancel. I filed incidents with TalkBack for each of these crasheds; their Incident IDs are TB24628054Z, TB24627541W, and TB24627239X. Has anybody else tried this? And did they run into the same problem?...

On my build 20010114 on Win2k I've succesfully went to the following URLs, which where taken from all DUPES of this bug. Both with Java disabled and Enabled. http://osdn.com/ http://www.msnbc.com http://search.perl.com/ http://www.zdnet.com/eweek http://home.excite.com http://www.kinigadner.com/intro.html http://www.siemens.com http://www.kiss.fi/ http://www.bestmat.be/en/occasbe.html http://www.xdrive.com/target.html?path=/company/main_download.html http://www.scee.com http://www.real.com http://www.vizzavi.co.uk http://www.lge.com.au/ http://www.maximonline.com http://www.m-w.com http://www.tellium.com http://cnnfn.cnn.com/2001/01/08/technology/wires/ibm_translate_wg http://www.thecounter.com/stats/2000/ http://ntv.ru

With the 2001011020 build on Windows NT, I'm still crashing when clicking the "Finance" link in www.vizzavi.co.uk

http://www.vizzavi.co.uk/finance/ on build 20010114 works fine for me. Try getting a newer build.

I just tried to go through the list of sites Henrik posted, and I got a crash on www.kiss.fi (incident TB24696077X). I'll try again inside a debugger and see if I can get a crash. Again, this is on Linux build 2001011208, without any plugins at all installed. I opened up Bugzilla in a window, went through Henrik's list, middle-clicked on each link, waited for the page to load (pressing STOP and RELOAD if it seemed stuck or had almost nothing on it), then closed the new window and went on to the next one in the list. Anybody have a similar setup that could try this?

This is a Windows NT only bug. If you are experiencing crashes running on Linux, then you should look for other plugin bugs or open a new one.

Interesting...I saw crashes on Flash pages about the time this bug was opened up, and assumed this was the cause. I'll poke around...

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

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