Reproduce procedure 1. launch mozilla 2. open preference:font 3. look at the font list 4. install a new font into the system 5. do 2 and 3 expect result- the newly installed font display in the font list and we can use it inside mozilla actual result- it won't show up. Roy mention there are a window's message WM_FONTCHANGE will be send to the window while the font installer install it, we should somehow pass that informtion to GFX and update the font cache. we need this so we can finish the "download font package on daemond" work. Roy- can you add this?

the code which receive window's message is at widget/src/windows/nsWindow.cpp

Roy's note: Windows will send application WM_FONTCHANGES msg to notify the font resource has been changed. Upon receiving the msg, we need to update the global fontlist and nsPresContext. Prefer using the XPCOM interface since the FontMetric is in gkgfxwin.dll and the widget is in gkwidget.dll. Here are the things need to do: - > add an interface method to nsFontEnumeratorWin service (say ::UpdateFontlist ()) to update the global fontlist. Use the nsGlobalFont.skip bit to identify the removed font(s) from the system. > use the similar mechanism found in nsFontMetricsWin.cpp/enumProc() to append the new font info into the global fontlist. - > update nsPresContext's device context font cache by using its existing RegisterCallback mechanism (nsPresContext::PreferenceChanged()) which is triggered by changing the "font.xxxx" in Preference <nsIPref>. (see layout/base/src/nsPresContext.cpp#440)

Focus on bug fix

More info on this: - we may or may not want to define a new METHOD nsIFontEnumerator::UpdateFontList() to update the global font list. This METHOD may be valid only for Windows. Other OSs may not want to iterate all the fonts; but only what's added/removed from the system. WM_FONTCHANGE msy only indicates "SOMETHING HAPPENED" to the system font. - instead of using nsGlobalFont.skip bit, it may be better if we define a new bit to identify the added/removed font(s) from the system. Then check for the CMAP of new font. Set the skip bit if the font has the same CMAP info or removed from the system. Options for updating the device context font cache: - Use nsIPref's Callback mechanism to globally notify the font change and calls the FlushFontCache() for every device context. - If DOMWindows contains any means of DC, then we can iterate each DOMWindows and call to update the font cache. - There may be a list of DC registered in global context. Get the list of DCs and call to update the font cache. - others.....

Due to interest from the very important embedding customer I'm setting --- or Future target milestone to 0.9.1

Changed QA contact to teruko@netscape.com.

PRUint8 skip; FONTSIGNATURE signature; int fonttype; + PRBool remove; Rather than adding yet another bolean attribute, what about just combining 'skip' and 'remove' in a bitfield, say 'flags'. Hence, other bits can be easily added as need arises. For example, I had hoped to set another 'truetype' bit to remember if a listed font is true type.

Need some synchronization: as part of the fix for bug 77265 (see the patch there), I added the flags data field as well.

rbs: I agree. I am not sure if putting 77265 to be a block for this bug is a good idea, but I did just to keep track of things. I also put 80756 as a blocker. shanjian owns it. I definitely need his fix. Can anyone /r= ?

I would suggest to replace all instance of : if ((gGlobalFonts[i].flags & ID_GLOBALFONT_SKIP) != ID_GLOBALFONT_SKIP) with if (!(gGlobalFonts[i].flags & ID_GLOBALFONT_SKIP)) That will be more concise and faster for CPU. r=shanjian.

brendan: can you /sr=?

making 74899 to be a blocker of this.

The patch at http://bugzilla.mozilla.org/showattachment.cgi?attach_id=34394 (last patch in this bug) seems to be out of date. Roy, can you cvs update, merge any conflicts, and attach a new one? Thanks, /be

TM to 0.9.2 per PDT triage (it's OK to check it in by Friday or after 0.9.1 branch is made).

brendan: can you /sr=?

There is missing sort (as per bug 77265) +NS_IMETHODIMP +nsFontEnumeratorWin::UpdateFontList() +{ ... + // set SKIP bit on for REMOVE fonts + SetGlobalFontSkipBit(); + >>>Call QuickSort here return NS_OK; } Also, you might want to update the sorting criteria so that removed fonts are placed last. The code for this is below (since at the beginning all fonts are available, i.e., the flag isn't set, the existing behavior will be retained). static int CompareGlobalFonts(const void* aArg1, const void* aArg2, void* aClosure) { const nsGlobalFont* font1 = (const nsGlobalFont*)aArg1; const nsGlobalFont* font2 = (const nsGlobalFont*)aArg2; // Sorting criteria is like a tree: // + Available fonts // + TrueType fonts // + non-Symbol fonts // + Symbol fonts // + non-TrueType fonts // + non-Symbol fonts // + Symbol fonts // + Removed fonts PRInt32 weight1 = 0, weight2 = 0; // computed as node mask if (font1->flags & NS_GLOBALFONT_REMOVE) weight1 |= 0x4; if (font2->flags & NS_GLOBALFONT_REMOVE) weight2 |= 0x4; if (!(font1->flags & NS_GLOBALFONT_TRUETYPE)) weight1 |= 0x2; if (!(font2->flags & NS_GLOBALFONT_TRUETYPE)) weight2 |= 0x2; if (font1->flags & NS_GLOBALFONT_SYMBOL) weight1 |= 0x1; if (font2->flags & NS_GLOBALFONT_SYMBOL) weight2 |= 0x1; return weight1 - weight2; } ============== On a different approach, I wonder what about just deleting the global list and calling InitializeGlobalFonts() again. Seems like that will do the job too (on Win32 at list). Is there any problems with that?

rbs: can you review for me? thanks

You also need to delete the gGlobalFonts[i].name [like in FreeGlobals()]: 193 if (nsFontMetricsWin::gGlobalFonts) { 194 //while all cmap is freed, gGlobalFonts's pointer should be freed 195 for (int i = 0; i < nsFontMetricsWin::gGlobalFontsCount; i++) { 196 delete nsFontMetricsWin::gGlobalFonts[i].name; 197 } 198 PR_Free(nsFontMetricsWin::gGlobalFonts); 199 nsFontMetricsWin::gGlobalFonts = nsnull; 200 gGlobalFontsAlloc = 0; 201 nsFontMetricsWin::gGlobalFontsCount = 0; 202 } While at it, the comment in line 194 is a bit misleading. You should remove it too. The 'name' that is used for the gFontMaps is not the same 'name' in gGlobalFonts[i].name. The one in gFontMaps comes from GetNAME() and is allocated _inside_ GetCMAP(). (If you try printing it for example, you will see that it is a very long string. It is an internal name that GDI uses to uniquely distinguish the font, and so it is a good one to use as the hashkey in gFontMaps.) Let me know how it goes after your testing.

Looks clean and tidy with no more leaks. r=rbs

brendan: can you /sr=? thanks

I think this is very critical for font download feature. Must have for moz0.9.2

+ for (int i = 0; i < nsFontMetricsWin::gGlobalFontsCount; i++) { + delete nsFontMetricsWin::gGlobalFonts[i].name; + } This code is being duplicated. Why don't you just use FreeGlobals in the file and reinitialize from there? + return ERROR_CALL_NOT_IMPLEMENTED; Shouldn't you be using NS_ERROR*? + PRBool fontInternalChange = PR_FALSE; + pPrefs->GetBoolPref("font.internaluseonly.changed", &fontInternalChange); + pPrefs->SetBoolPref("font.internaluseonly.changed", !fontInternalChange); you refer to this flushing the cache but I don't see the support code anywhere in the tree to monitor that change.

>Why don't you just use FreeGlobals in the file >and reinitialize from there? We don't want to destroy all the hashtables and release other data. We should only re-create the global font list. Thus InitializeGlobalFonts(dc)) call; not InitGlobals(void). >+ return ERROR_CALL_NOT_IMPLEMENTED; >Shouldn't you be using NS_ERROR*? I believe NS_ERROR_NOT_IMPLEMENTED is an appropriate "ERROR CODE" since other platforms may want to support in the future. >you refer to this flushing the cache but I don't see the support code anywhere >in the tree to monitor that change. nsPrefContext::Init() registers the font callback by using mPrefs->RegisterCallback("font.", PrefChangedCallback, (void*)this); which calls mDeviceContext->FlushFontCache(); in the end.

Sorry, I didn't realize I was using ERROR_CALL_NOT_IMPLEMENTED. It should read NS_ERROR_NOT_IMPLEMENTED instead of ERROR_CALL_NOT_IMPLEMENTED. I'll post a patch in a moment.

blizzard: can you super review? thanks

sr=blizzard

a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)

checked-in

Changed QA contact to ylong@netscape.com.

ylong: Can you verify this bugs against today's trunk build? We had a patch checked-in for 89493 last night which may affect this feature. Here is what you need to do: - Pick a machine without a CJK font (say NT-En without Ja-GlobalIME) - launch browser - visit Ja site (say www.netscape.com/ja) - Ja font download dlgbox should come up. You also see that the Ja page is shown ? marks and un-readable text. This is ok because you don't have the ja font just yet. - After finishing the font download, the Ja page should AUTOMATICALLY update and display Ja text correctly. You shouldn't need to press "Reload" nor shut down the browser. If it doesn't update/display correctly AUTOMATICALLY, then this feature is broken. Thanks

I checked it on 07-11-07 trunk build / WinNT-En without CJK font: 1. If I go http://home.netscape.com/ja or http://home.netscape.com/zh/tw will pops-up a dialog for downloading Japanese or Chinese, which are correctly. However, if I go: http://home.netscape.com/ko or http://home.netscape.com/zh/cn will pops-up 2 downloading dialogs - Korean and Japanese or Simp. Chinese and Japanese which are a little strenge. 2. > - After finishing the font download, the Ja page should AUTOMATICALLY update and display Ja text correctly. You shouldn't need to press "Reload" nor shut down the browser. -- I checked it on http://www.netscape.com/ko, after I finish downloading, the Kerean characters apprear automatically but has characters overwrrapping. If I reload the page, then will display fine.

ReOpening. This is no longer working due to 89493 (see comment by Marc Attinasi 2001-07-09) I used 20010724 trunk dbg build. Setting the milestone to 0.9.4

91250 is checked in.

Fixed verified on 09-26 branch build / WinNT-EN.