233540 - 1.6 crashes at http://meteo.ec.gc.ca/forecast/city_f.html?wjo

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Yukinoroh[:Yukinoroh]]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.6) Gecko/20040113

I've been unable to view that page with Mozilla 1.6.
It works with 1.3 (haven't checked the other versions though).

Reproducible: Always
Steps to Reproduce:
1. Go at the address
Actual Results:  
Crash.

Expected Results:  
Render the page.

"Unknown mdoule" (That's what the crash window says)

Comment 1

[WD]

WFM Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040206
Firefox/0.8

Comment 2

[Mats Palmgren (inactive)]

WFM, Mozilla 1.6 and 2004-02-08-08 trunk Linux

Comment 3

[Yukinoroh[:Yukinoroh]]

Same problem with the following address :
http://www.zdnet.fr/actualites/opinions/0,39020797,39137464,00.htm

(I thouhgt of putting it here rather than creating a new bug - I get the same
symptoms after all.)

Comment 4

[Yukinoroh[:Yukinoroh]]

This one crashes as well :
http://elsap1.unicaen.fr/dicosyn.html

(I used to be able to access all these sites with 1.3 ... If you guys need me to
test something please tell me, otherwise I think I'll be downgrading to the
lower version until they stop crashing... Is 1.6 that unstable on your side too?)

Comment 5

[WD]

Try an uninstall, install to a clean directory, and make a new profile.

Comment 6

[Yukinoroh[:Yukinoroh]]

Tried it, didn't work.

I have the same problem on
http://www.uqac.ca/anime/archives/2002-2003/5a7_minicosplay.html
as well as when opening my Netscape 4.5 bookmark file with File, Open (in
Mozilla of course).

I might get some sort of pattern here - all of these pages are in or contain
French, and I am under a japanese Windows95b environment. It might actually be
linked with bug #233549 that I reported recently.

Comment 7

[Yukinoroh[:Yukinoroh]]

And I actually crash when trying to view bug #233549 o_O;

It might be because it contains the faulty characters. Though, I'm not sure on
wether I had viewed it after posting it, yesterday.

Comment 8

[Yukinoroh[:Yukinoroh]]

I've just tried editing the page of bug #233549 in different ways with notepad,
and tried loading it in Mozilla :

Whole page : crashes
Whole page without the text in Japanese : works
Only the text in Japanese: works
Everything after the text in Japanese is deleted : crashes
Everything before the text in Japanese is deleted : works
(when deleting, I was only tounching what was inside the body tags)

I get a similar pattern while deleting in the other files that were crashing -
it doesn't seems to just crash on a specific character, but on a specific
character after a certain number of characters.

Comment 9

[Yukinoroh[:Yukinoroh]]

Attachment: My test

Sending the test I did

Comment 10

[Jungshik Shin]

So, there's something Win 95 (or Win 9x/ME) specific. Can anyone test it on Win
98/ME? Reporter, is your Win 95 Japanese? 

Comment 11

[Jungshik Shin]

sorry for bug spam. I forgot to add myself to cc.

Comment 12

[Yukinoroh[:Yukinoroh]]

Yes, that is right, my Windwos 95 is Japanese.

Comment 13

[Yukinoroh[:Yukinoroh]]

I crash there as well (and all the time) :
http://hakusyu.com/goods/index.html
This is getting me puzzled; it is not a French page, but a Japanese one...

Comment 14

[Jungshik Shin]

This bug is tough to fix because no active mozilla developer has Win95 JA. There
are some Win95J-specific code paths and we need Win95 JA to debug it. 

In addition, somebody has to try problematic sites with non-Japanese Win95, Win
98/ME JA, and non-Japanese Win 98/ME to make sure that Win9x-JA-specific code
paths are to blame. Why don't you ask for  help at the Japanese bugzilla at
http://bugzilla.mozilla.gr.jp ? 

It seems to be 'real' so that I'm confirming. 

Comment 15

[Yukinoroh[:Yukinoroh]]

Attachment: Test #2

I did some testing with that last page, so I'm sending my test.

main2.htm crashes
main3.htm is okay

What I did is to delete the ideographic space character (U+3000) that was
filling the cells.

Comment 16

[Yukinoroh[:Yukinoroh]]

Hm, I'm not sure I'm *that* at ease with Japanese yet to be able to have a
technical talk ^^,,, I'll give it a try, though.

BTW, I have French Win95 here - it's just not installed yet. I usually have the
two OS installed, but I've recenlty recovered from a hard disk crash so it's not
ready...

Comment 17

[Yukinoroh[:Yukinoroh]]

The pages don't crash in Mozilla 1.5.

Comment 18

[rbs]

jshin, you can force the 'A' codepath with 'return 1' in UseAFunctions() in
nsGfxFactoryWin.cpp.

With that, Mozilla behaves as on a Win95-Japanese box and it then becomes a
matter of differences with the underlying OS libs.

I tried the setting (on my Win2K-EN box) but I am not reproducing the crashes.

Comment 19

[u115577]

I'm running Mozilla Firebird 0.7 on Windows 95 in Japanese. When I upgraded to
Firefox 0.8, it crashed on some pages, and I found this bug. All urls mentioned
above cause Firefox 0.8 to crash. The only thing I can say is that this is
regression after Firebird 0.7.

Mozilla/5.0 (Windows; U; Win95; ja-JP; rv:1.5) Gecko/20031007 Firebird/0.7
Mozilla/5.0 (Windows; U; Win95; ja-JP; rv:1.6) Gecko/20040210 Firefox/0.8

I encounter crash on this page (Japanese site):
http://www.onlinesofts.com/

Comment 20

[shiten]

The pages don't crash in Mozilla Firefox 0.8(WinME/Japanese).

Comment 21

[Yukinoroh[:Yukinoroh]]

Still crashes on Mozilla 1.72 (Win95/Japanese)

Comment 22

[Jungshik Shin]

Can you take a look?

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

# WFM 2004082007-trunk/Win2k, WinMe(VMWare)

Can you test on Win95-JA?

Comment 24

[Jungshik Shin]

Nakano-san, thank you for testing.
Can you try what rbs suggested in comment #18? (see also my comment #14). That
way, you can emulate Win 95/J even on Win 2k/XP, I think. 
I really should buy VMware so that I don't have to reboot my machine to Windows.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Can you try what rbs suggested in comment #18?

O.K. I will test tomorrow.

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I cannot reproduce to crash, but hung up.(on win2k)
The test build is here.
http://www.d-toybox.com/mozilla/testbuilds/bug-org233540.zip

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I can reproduce with win2k + above test build in Amazon.co.jp.
1. Go to http://www.amazon.co.jp/
2. Type "word" in the search input box.
3. Push "Go" button.

Comment 28

[u115577]

We are in the process of identifying the cause of this bug on Bugzilla-jp.
Apparently, a particular character may cause Mozilla/Firefox to crash.
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=4012

Comment 29

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.0

Taking.
This is a regression of bug 205387.
When documents include the ignorable characters, some nsFontMetricsWinA methods
will attempt to cast mLoadedFonts[0] (aka gFontForIgnorable) to nsFontWinA*.
But gFontForIgnorable never becomes an instance of nsFontWinA even if
UseAFunctions() returns TRUE.

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 190868 [details] [diff] [review]
Patch rv1.0

Ere:

Please review it.

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

gfx code will be removed in 1.9 cycle. We should fix this bug in 1.8 branch.

Comment 32

[timeless]

Comment on attachment 190868 [details] [diff] [review]
Patch rv1.0

>Index: gfx/src/windows/nsFontMetricsWin.cpp
>@@ -368,20 +436,42 @@ InitGlobals(void)
>+  if (UseAFunctions()) {
>+    nsFontWinSubstituteA* font = new nsFontWinSubstituteA(gIgnorableCCMapExt);
>+    gFontForIgnorable = font;

>+    font->mSubsets = (nsFontSubset**)nsMemory::Alloc(sizeof(nsFontSubset*));
>+    if (!font->mSubsets) {
afaict you'll leak gFontForIgnorable the next time you come through this code
if you run out of memory this time.
>+      FreeGlobals();
>+      return NS_ERROR_OUT_OF_MEMORY;
>+    }

>+  } else {
>+    gFontForIgnorable = new nsFontWinSubstitute(gIgnorableCCMapExt); 
>+    if (!gFontForIgnorable) {
same concern
>+      FreeGlobals();
>+      return NS_ERROR_OUT_OF_MEMORY;

>   //register an observer to take care of cleanup
>   gFontCleanupObserver = new nsFontCleanupObserver();
this assertion needs to go away, i know it isn't yours, but it either needs to
die or have a bug filed to arrange for it to die. you can not assert oom won't
happen.
>   NS_ASSERTION(gFontCleanupObserver, "failed to create observer");
>   if (gFontCleanupObserver) {
>     // register for shutdown
>     nsresult rv;

>@@ -5093,16 +5178,24 @@ nsFontMetricsWinA::FindGlobalFont(HDC aD
> nsFontWinSubstituteA::nsFontWinSubstituteA(LOGFONT* aLogFont, HFONT aFont,
>   PRUint16* aCCMap) : nsFontWinA(aLogFont, aFont, aCCMap)
> {
>+  mIsForIgnorable = FALSE;
we normally use PRBool and PR_TRUE/PR_FALSE ...
so in fact you're using the wrong kind as seen by the declaration below:

>Index: gfx/src/windows/nsFontMetricsWin.h
>+private:
>+  PRBool mIsForIgnorable;

Comment 33

[Masatoshi Kimura [:emk]]

Comment on attachment 190868 [details] [diff] [review]
Patch rv1.0

> afaict you'll leak gFontForIgnorable the next time you come through this code
> if you run out of memory this time.
There is no concern because FreeGlobals() will free gFontForIgnorable
and destructor will free mSubsets. Wrong?
> same concern
We can't leak gFontForIgnorable which isn't even created.

I have changed other points.

Comment 34

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.1

Comment 35

[Mike Schroepfer]

Won't block beta but will take if done one time.

Comment 36

[timeless]

Comment on attachment 190998 [details] [diff] [review]
Patch rv1.1

>Index: gfx/src/windows/nsFontMetricsWin.cpp
>+  //when fontSubstitute declare it support certain char, no need to check subset,
'declares' it 'supports' 'a' certain char
>+  // since we have one and only one subset.
>+  virtual PRBool HasGlyph(PRUnichar ch) {return PR_TRUE;};

>@@ -368,32 +436,56 @@ InitGlobals(void)

>+    font->mSubsets = (nsFontSubset**)nsMemory::Alloc(sizeof(nsFontSubset*));
>+    font->mSubsets[0] = nsnull;
>+    nsFontSubsetSubstitute* subset = new nsFontSubsetSubstitute(TRUE);
>+    if (!subset) {
what sets mSubsetsCount to 0?
>+      FreeGlobals();
>+      return NS_ERROR_OUT_OF_MEMORY;
>+    }
>+    font->mSubsetsCount = 1;
>+    font->mSubsets[0] = subset;

>@@ -5093,16 +5180,24 @@ nsFontMetricsWinA::FindGlobalFont(HDC aD
> nsFontWinSubstituteA::nsFontWinSubstituteA(LOGFONT* aLogFont, HFONT aFont,
>   PRUint16* aCCMap) : nsFontWinA(aLogFont, aFont, aCCMap)
> {
use an initializer for this:
>+  mIsForIgnorable = PR_FALSE;
>+  memset(mRepresentableCharMap, 0, sizeof(mRepresentableCharMap));
>+}
>+
>+nsFontWinSubstituteA::nsFontWinSubstituteA(PRUint16* aCCMap) :
>+  nsFontWinA(NULL, NULL, aCCMap)
>+{
use an initializer for this:
>+  mIsForIgnorable = PR_TRUE;
>   memset(mRepresentableCharMap, 0, sizeof(mRepresentableCharMap));
> }

>Index: gfx/src/windows/nsFontMetricsWin.h
>@@ -407,21 +407,26 @@ public:
> class nsFontWinSubstituteA : public nsFontWinA
>+private:
>+  PRBool mIsForIgnorable;
> 
>   //We need to have a easily operatable charmap for substitute font
either 'for substitute fonts' or 'for a substitute font', i'm not sure which
you mean.

>   PRUint32 mRepresentableCharMap[UCS2_MAP_LEN];
> };

please post a new patch and sr?rbs/neil

Comment 37

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.2

> what sets mSubsetsCount to 0?
I added initializer in nsFontWinA ctor.
> either 'for substitute fonts' or 'for a substitute font', i'm not sure which
you mean.
I don't know either because shanjian wrote it (not mine).
Formerly, there were only one nsFontWinSubstitute(A) instance because it is
only used for missing glyphs.
But nsFontWinSubstitute(A) is reused for gFontForIgnorable, so I think perhaps
'for substitute fonts' is better now.

Comment 38

[rbs]

Comment on attachment 191132 [details] [diff] [review]
Patch rv1.2

sr=rbs

Some nits that you can address at checkin.

> > what sets mSubsetsCount to 0?
> I added initializer in nsFontWinA ctor.

You don't need that because it uses NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW,
i.e., its |new| does a memset to as well.

+  if (UseAFunctions()) {

Personal preference: I generally like when it is |if () { small code } else {
long, tedious code...}|.

Comment 39

[Masatoshi Kimura [:emk]]

Attachment: Patch rv1.3

Very low risk. This affects only Win95-JA specific codepath.
And this is a fix for crasher.

Comment 40

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Mozilla Japan think that this bug should be fixed 1.8 branch.

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I will check-in the patch at tonight(JST).

Comment 42

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.

Comment 43

[Yukinoroh[:Yukinoroh]]

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