Last Comment Bug 92596 - better font download handler on window
: better font download handler on window
Status: RESOLVED WONTFIX
[correctness]
: intl
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: x86 Windows NT
: P2 normal (vote)
: Future
Assigned To: Nobody; OK to take it and work on it
: Teruko Kobayashi
Mentors:
http://www.mozilla.org/projects/intl/...
Depends on: 352049
Blocks:
  Show dependency treegraph
 
Reported: 2001-07-27 10:56 PDT by Frank Tang
Modified: 2014-04-26 03:08 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (include add two new files under mozilla/intl/locale/src/windows (11.35 KB, patch)
2001-07-27 17:45 PDT, Frank Tang
no flags Details | Diff | Review
v2 of the patch (do not include the calling part yet) (13.85 KB, patch)
2001-10-25 16:02 PDT, Frank Tang
no flags Details | Diff | Review

Description Frank Tang 2001-07-27 10:56:49 PDT
we should implement better font download handler on window, it should 
1. download all different fonts, not just CJK, we should add
cyrillic, baltic, thai, vietnamese, central european, arabic, hebrew, greek,
trukish, 

2. required now shotdown.
Comment 1 Frank Tang 2001-07-27 12:06:13 PDT
we could implement this by feeding page with meta charset to ie
we can fine ie from 
regestry 
to find the path of Internet explorer
we first look at 

ieclsid  value = "HKEY_CLASSES_ROOT:InternetExplorer.Application:CLSID"
and use that value to find in 
"HKEY_CLASSES_ROOT:CLSID:$ieclsid:LocalServer32"
Comment 2 Frank Tang 2001-07-27 12:11:31 PDT
I think we should implement this code in
mozilla/xpfe/components/winhook
as nsWinFontDownloadHandler

Comment 3 Frank Tang 2001-07-27 17:45:35 PDT
Created attachment 43857 [details] [diff] [review]
patch (include add two new files under mozilla/intl/locale/src/windows
Comment 4 Frank Tang 2001-08-06 11:46:15 PDT
what happen to this patch?
Comment 5 Frank Tang 2001-08-06 17:16:43 PDT
roy can you try this patch in your system ? and also code review it ?
Comment 6 Roy Yokoyama 2001-08-07 14:51:54 PDT
ftang: I have few questions and suggestions
QUESTIONS:
1) what is >#ifdef XP_WIN  ??? Only define in MS Windows XP platform?
2) Need error checking.  Instead of :
   +    nsWinFontPackageHandler* fp = new nsWinFontPackageHandler;
   +    if(fp->Work())
   +    {
   How about:
   +    nsWinFontPackageHandler* fp = new nsWinFontPackageHandler;
   +    if (!fp) 
   +      return NS_ERROR_OUT_OF_MEMORY;
   +    if(fp->Work())
   +    {
3) Sorry, Frank.  I know you came to me and ask about this; but
   please use CreateProcess() API instead of obsolete WinExec().
4) Should we use nsCString instead of nsCAutoString?  nsCAutoString,
   by definition, uses stack based allocation. 
   +class nsWinFontPackageHandler : public nsIFontPackageHandler
   +{
   .........
   +  nsCAutoString mIEPath;
   +  nsCAutoString mIEVersion;
   +};
5) Can we not to use char buffer[4096]?  I rather like to see 
   XPIDLString, nsCString, etc. We may want to ask Scott Collins for his 
opinion.
   +void nsWinFontPackageHandler::InitIEPath()
   +{
   +    ...
   +    char buffer[4096] = {0};
   +    DWORD len;  

SUGGESTIONS:
1) Can we use more generic away?  Instead of Windows platform centric, 
   I believe, we should support other platforms as well.  Mac and Unix
   would like to have this feature. 
2) Instead of rely on IE, we should have our own font files to download. 
   Then we don't need to shutdown Windows and restart. (required by 
   the current GlobalIME implementation.) 

   
Comment 7 Frank Tang 2001-08-21 15:28:59 PDT
move to m94
Comment 8 Frank Tang 2001-08-30 17:43:45 PDT
move to m0.9.5
Comment 9 msanz 2001-09-07 11:19:17 PDT
moving to 0.9.4 nsbranch+ because of dependencies with bugscape 6879. At least
the  JA part of it should be fixed,
Comment 10 Jaime Rodriguez, Jr. 2001-09-10 09:28:19 PDT
Msanz - Should this still be assigned to FTang? Will he be able to fix it in
time for this release? Should we reassign or later this one?
Comment 11 msanz 2001-09-10 22:54:22 PDT
leave it until he is back. There is also another dependency that I just found
with bugscape 5393. I want him to asses the whole group.
Comment 12 Asa Dotzler [:asa] 2001-09-15 12:05:32 PDT
0.9.4 is out the door.
Comment 13 Jaime Rodriguez, Jr. 2001-09-21 09:28:58 PDT
Adding correctness Status Whiteboard, correct/expected behavior does not occur.
Comment 14 selmer (gone) 2001-09-24 18:30:21 PDT
Why would we not ship if this isn't fixed?  We obviously didn't have this in
6.1, so it must be possible to ship...
Comment 15 Jaime Rodriguez, Jr. 2001-09-25 10:07:58 PDT
Agree with Selmer's comments. PDT-
Comment 16 Frank Tang 2001-09-25 17:12:36 PDT
move nsbranch+ to nsbranch-
Comment 17 Frank Tang 2001-10-08 11:31:11 PDT
move to m0.9.6
Comment 18 Frank Tang 2001-10-16 06:25:13 PDT
ok, design spec put in http://www.mozilla.org/projects/intl/fontdow2.html
Comment 19 Frank Tang 2001-10-18 14:29:22 PDT
have problem with CreateProcess() 

It work if I use 
          nsCAutoString command;
          command.Assign(mIEPath);
          command.Append(" -k ");
          command.Append(tmpfile);
          const char* pCmd = command.get();
          UINT ret = WinExec(pCmd, SW_HIDE);

But it does not launch IE if I use the following:


          nsCAutoString command;
          command.Assign(mIEPath);
          command.Append(" -k ");
          command.Append(tmpfile);
          const char* pCmd = command.get();
          STARTUPINFO startupInfo;
          PROCESS_INFORMATION processInfo;
          ::memset(&startupInfo, 0, sizeof(startupInfo));
          startupInfo.cb = sizeof(startupInfo);
          startupInfo.dwFlags = STARTF_USESHOWWINDOW;
          startupInfo.wShowWindow = SW_HIDE;
          WORD ret = ::CreateProcess(mIEPath, (char*) pCmd, NULL, NULL,
                         FALSE, 
                         CREATE_DEFAULT_ERROR_MODE ,
                         NULL, NULL,
                         &startupInfo, 
                         &processInfo);

Roy: What should i do ?
Comment 20 Frank Tang 2001-10-25 16:02:50 PDT
Created attachment 55143 [details] [diff] [review]
v2 of the patch (do not include the calling part yet)
Comment 21 Frank Tang 2001-10-25 16:05:57 PDT
ok, here is the new window font package installer. I will work on that part.

roy- can you review the patch. To see that take effect, apply the patch and also 
change the following temp code in your local tree:

Z:\mozilla\intl\locale>cvs diff -u src
cvs server: Diffing src
Index: src/nsFontPackageService.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/locale/src/nsFontPackageService.cpp,v
retrieving revision 1.6
diff -u -r1.6 nsFontPackageService.cpp
--- nsFontPackageService.cpp    2001/10/02 21:30:26     1.6
+++ nsFontPackageService.cpp    2001/10/25 22:59:52
@@ -79,7 +79,8 @@
     nsresult rv;

     // create default handler
-    mHandler = do_CreateInstance("@mozilla.org/locale/default-font-package-hand
ler;1", &rv);
+    //mHandler = do_CreateInstance("@mozilla.org/locale/default-font-package-ha
ndler;1", &rv);
+    mHandler = do_CreateInstance("@mozilla.org/locale/win-font-package-handler;
1", &rv);
     if (NS_FAILED(rv)) return rv;
   }
   return mHandler->NeedFontPackage(aFontPackID);


Probably we should put "default-font-package-handler;1" into a pref?

the design paper of this work can be found at
http://www.mozilla.org/projects/intl/fontdow2.html
Comment 22 Frank Tang 2001-10-26 15:57:02 PDT
any comment about the review, roy?
Comment 23 Roy Yokoyama 2001-10-29 10:56:19 PST
ftang: I have finished applying the patch on my WinXP; but I need to
test little more.   I guess your patch looks reasonable if we decided to
use IE for downloading the font.  (It's against my wish of being Mozilla rely
on IE .....)
Comment 24 Roy Yokoyama 2001-10-29 16:31:24 PST
ftang: it works fine on my WinXP. As your document stays we need
ExitProcess() to end the process created by CreateProcess()
Comment 25 Arthur Barrett 2001-11-08 14:02:28 PST
I might be missing something, but doesn't implementing this mean that 
installing IE5 is as pre-requisite to installing Mozilla (if you want semi-auto 
font downloads for international pages)?
Comment 26 timeless 2001-11-08 14:55:27 PST
Comment on attachment 55143 [details] [diff] [review]
v2 of the patch (do not include the calling part yet)

+  NS_ASSERTION(NS_SUCCEEDED(res), "cannot get IE version");

You can't ASSERT for this.  The absense of IE must at most be a non fatal
warning.
Comment 27 nhottanscp 2001-11-20 13:51:14 PST
move to 0.9.8
Comment 28 Frank Tang 2002-01-16 18:07:52 PST
mass move to m9.9
Comment 29 Frank Tang 2002-02-13 17:57:05 PST
mark this as future for now.
Comment 30 Frank Tang 2005-03-01 23:57:12 PST
what a hack. I have not touch mozilla code for 2 years. I didn't read these bugs
for 2 years. And they are still there. Just close them as won't fix to clean up.
Comment 31 Travis Chase 2005-03-02 03:52:54 PST
Mass Re-open of Frank Tangs Won't fix debacle. Spam is his responsibility not my own
Comment 32 Travis Chase 2005-03-02 04:19:21 PST
Mass Re-assinging Frank Tangs old bugs that he closed won't fix and had to be
re-open. Spam is his fault not my own
Comment 33 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-10 12:22:49 PDT
bug 352049 killed the font download dialog, marking all bugs about it wontfix.

Note You need to log in before you can comment on or make changes to this bug.