The has been a regression from the fix that was landed for bug 63965. From the attachment 28633 [details] [diff] [review] on that bug, LoadGlobalFont() is not behaving as LoadFont(): ! //return LoadFont(aDC, gGlobalFonts[i].name); ! return LoadGlobalFont(aDC, &(gGlobalFonts[i])); I will attach screenshots of the above URL to demonstrate the problem.

I have verified that the huge glyphs come the 'Lucida Sans Unicode' font, i.e., when FONT_HAS_GLYPH() returns true, it is the font currently picked in the global list. The primary difference that I can see with LoadGlobalFont() is that it is using 'nsFontWinA' whereas 'nsFontWin' is used elsewhere: + nsFontWin* + nsFontMetricsWin::LoadGlobalFont(HDC aDC, nsGlobalFont* aGlobalFontItem) + { + HFONT hfont = ::CreateFontIndirect(&(aGlobalFontItem->logFont)); + + if (hfont) { + if (mLoadedFontsCount == mLoadedFontsAlloc) { + int newSize = 2 * (mLoadedFontsAlloc ? mLoadedFontsAlloc : 1); + nsFontWinA** newPointer = (nsFontWinA**) PR_Realloc(mLoadedFonts, + newSize * sizeof(nsFontWinA*)); + if (newPointer) { + mLoadedFonts = (nsFontWin**) newPointer; + mLoadedFontsAlloc = newSize; + } + else { + ::DeleteObject(hfont); + return nsnull; + } + } ... }

different from bug 76826 and the backed out code for bug 46863 ?

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

Zooming to 10% doesn't change the size of the huge elements, but other elements change.

nsFontWinA is supposed to be used only with nsFontMetricsWinA, which was created to work around a bug in Japanese Windows 95. However, the code Roger mentions above is only dealing with the size of nsFontWinA*, so it is not really a big deal (since both nsFontWin* and nsFontWinA* are 4 bytes). The bug is probably somewhere else.

Right, the code is allocating 'nsFontWinA** newPointer' and then re-casting. I had overlooked the cast: + mLoadedFonts = (nsFontWin**) newPointer; So as per your comments: there is this bad usage of nsFontWinA + bug elsewhere.

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

Why is the severity "normal". This should be a blocker as it makes the browser unusable (the first two sites I tried were jus unreadable). From what I've seen before moving back to yesterday's build, I am sure it breaks some smoketests, as well.

I found the problem. Logfont from global font is used directly. However, I forgot font size (lfHeight, lfWidth) is also in LOGFONT structure. I will post a patch right away.

This bug might cause serious problem, please help review the bug. Erik, Could you give a sr=? Fix explaination: Same function "FillLogFont" is used for "LoadGlobalFont". I assigned lfCharset and other stuff from global later. And this new logfont replace the logfont from global item.

-> cvs diff -u ?

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

I have read Shanjian Li's comments on bug 63965, particularly those dated "Shanjian Li 2001-03-22 13:46", "Shanjian Li 2001-03-23 13:20", and "Shanjian Li 2001-04-03 08:42". Now, I understand the motivations of the addition of the LoadGlobalFont() function, especially as I remember how I was bitten in bug 46438 by the failures of GetGlyphOutline() due to similar weirdnesses. To better understand what is going on here, one has to go back and start with bug 63965. In the previous code, i.e., before the fix for that bug, the global list of fallback fonts was constructed by asking the GDI to enumerate the list of fonts available. However, the information (i.e, logfont) that was returned by the GDI during this enumeration wasn't used later on when instantiating the font. Rather, only the name of the font was added to a temporary logfont, together with some other default initializations, and the resulting logfont was passed to the GDI to instantiate a hfont and retrieve the associated cmap, etc. As bug 63965 has shown, there were troubles with this approach. And the investigation carried there showed that the GDI wasn't happy, and also wanted some other attributes (e.g., the charset) that it returned during its earlier enumeration. So Shanjian Li's fix for that bug has consisted in exactly passing these attributes within the newly added LoadGlobalFont(). Also, a change was made in the way the global list is stored: the TrueType fonts were placed at the head of the list to guarantee that FindGlobalFont() will always try the TrueType fonts first. Unfortunately, in the course of this fix, Shanjian Li forgot to explicitly specify some other varying attributes that are needed to produce the desired stylistic effects when instantianting a hfont, e.g, the font size (lfHeight, lfWidth). So the current proposed fix addresses these ommissions by filling the missing attributes. r=rbs, based on this evaluation and understanding, though I haven't tested the patch as I am currently behing a restricted firewall.

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

Your understanding is absolutely correct. LOGFONT contains both information applied to font as a whole and to instantiation of a font. The second category of infor (size, style) should be refilled when a font is loaded to system for use.

sr=blizzard

Tested the patch and it worked.

fix checked in.

verified fixed on windows commercial build 2001-04-24-06

Re-opening. The handling of the font-weight was missed during the fix.

Here is the patch to fix the omission: Index: nsFontMetricsWin.cpp =================================================================== RCS file: /cvsroot/mozilla/gfx/src/windows/nsFontMetricsWin.cpp,v retrieving revision 3.111 diff -u -r3.111 nsFontMetricsWin.cpp --- nsFontMetricsWin.cpp 2001/06/08 23:36:11 3.111 +++ nsFontMetricsWin.cpp 2001/06/15 07:50:21 @@ -1920,7 +1920,9 @@ LOGFONT logFont; HFONT hfont; - FillLogFont(&logFont, aGlobalFontItem->logFont.lfWeight); + PRUint16 weightTable = LookForFontWeightTable(aDC, aGlobalFontItem->name); + PRInt32 weight = GetFontWeight(mFont->weight, weightTable); + FillLogFont(&logFont, weight); logFont.lfCharSet = aGlobalFontItem->logFont.lfCharSet; logFont.lfPitchAndFamily = aGlobalFontItem->logFont.lfPitchAndFamily; strcpy(logFont.lfFaceName, aGlobalFontItem->logFont.lfFaceName);;

Actually, the fix is needed in two places: nsFontMetricsWin::LoadGlobalFont() nsFontMetricsWinA::LoadGlobalFont()

rbs- shanjian just had a new baby last week. he will be on vacation and off line for two weeks to take care his new baby. He won't read email or bug report untill probably end of June. can you take care this one for him? I am willing to code review for you. Not 100% clear about your last patch. You mention that need to be changed in two place. Can you put a cvs diff -u here ?

change status from "important to mozilla 0.9.2" to "shanjian on vacation, wait rbs to submit small fix"

What the patch is doing is replacing "aGlobalFontItem->logFont.lfWeight" (a system default) by the actual font-weight obtained during style resolution. The bug can be easily reproduced using a markup like: <span style="font-weight:bold"> &NNNN; </span> where "NNNN" is the Unicode number of an uncommon character, e.g., &FBAC;

ftang, what is up with this one? It would be nice to avoid the hassle of a late approval because that would tie me to keeping a branch tree until the r/sr/a etc is complete. This regression happens whenever there is an i18n character that is likely to send you to FindGlobalFont() -- once there 'font-weight:bold' wouldn't work.

Re-assigning to ftang for checkin - Due to the time-difference, I will not be in a suitable position to checkin given tommorrow's deadline for wrapping m0.9.2.

move to moz0.9.3 nsbranch I think we should got sr and check into trunk. and we should land this to moz0.9.2 after mozilla realease it.

r=ftang

sr=blizzard

we should land to trunk first.

ask yokoyama to help land to trunk

checked into trunk.

adding vtrunk keyword to indicate the bug has been fixed on the trunk. Bug left open for tracking checkin to branch (nsbranch) when appropriate. Once bug has been fixed on the branch also, pls remove vtrunk keyword.

rbs, can you test this on branch?

I have hard time to create a good test case which will show the difference with, or without the patch. So.... I think we should not land this into moz0.9.2 branch for now. rbs- can you create a good test case that show the difference? if you can, can you attach it to the bug ?

Sorry, I have other things in my plate.

rbs- I am thinking moving this out of our m9.2 branch list. Since it is already checked into trunk, we can mark it fixed right now. What is the worst case if we don't fix this in the branch?

Frank, as I reported earlier, if you have any i18n text, say <p style="font-weight:bold"> an i18n text </p> [so that some glyphs have to be fetched for sure with FindGlobalFont()] then these glyphs wont be bold without the fix.

rbs, can you attach your test cases (which produce the gif file you attached ) as attachment? Thanks

It is available from the URL field: http://lxr.mozilla.org/seamonkey/source/layout/mathml/tests/symbol.html

Not going on the branch. Marking fixed.

Marking verified per last comments.