better font download handler on window

RESOLVED WONTFIX

Status

()

Core
Internationalization
P2
normal
RESOLVED WONTFIX
16 years ago
3 years ago

People

(Reporter: Frank Tang, Unassigned)

Tracking

({intl})

Trunk
Future
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [correctness], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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.

Updated

16 years ago
Keywords: intl
QA Contact: andreasb → teruko
(Reporter)

Comment 1

16 years ago
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"
(Reporter)

Comment 2

16 years ago
I think we should implement this code in
mozilla/xpfe/components/winhook
as nsWinFontDownloadHandler

(Reporter)

Comment 3

16 years ago
Created attachment 43857 [details] [diff] [review]
patch (include add two new files under mozilla/intl/locale/src/windows
(Reporter)

Comment 4

16 years ago
what happen to this patch?
(Reporter)

Comment 5

16 years ago
roy can you try this patch in your system ? and also code review it ?
Status: NEW → ASSIGNED

Comment 6

16 years ago
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.) 

   
(Reporter)

Comment 7

16 years ago
move to m94
Target Milestone: --- → mozilla0.9.4
(Reporter)

Comment 8

16 years ago
move to m0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
(Reporter)

Updated

16 years ago
Priority: -- → P2

Comment 9

16 years ago
moving to 0.9.4 nsbranch+ because of dependencies with bugscape 6879. At least
the  JA part of it should be fixed,
Keywords: nsbranch+
Target Milestone: mozilla0.9.5 → mozilla0.9.4

Comment 10

16 years ago
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

16 years ago
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

16 years ago
0.9.4 is out the door.
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 13

16 years ago
Adding correctness Status Whiteboard, correct/expected behavior does not occur.
Whiteboard: [correctness]

Comment 14

16 years ago
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

16 years ago
Agree with Selmer's comments. PDT-
Whiteboard: [correctness] → [correctness], PDT-
(Reporter)

Comment 16

16 years ago
move nsbranch+ to nsbranch-
Keywords: nsbranch+ → nsbranch-
(Reporter)

Comment 17

16 years ago
move to m0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
(Reporter)

Updated

16 years ago
Blocks: 104056
(Reporter)

Comment 18

16 years ago
ok, design spec put in http://www.mozilla.org/projects/intl/fontdow2.html
(Reporter)

Comment 19

16 years ago
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 ?
(Reporter)

Updated

16 years ago
Attachment #43857 - Attachment is obsolete: true
(Reporter)

Comment 20

16 years ago
Created attachment 55143 [details] [diff] [review]
v2 of the patch (do not include the calling part yet)
(Reporter)

Comment 21

16 years ago
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
(Reporter)

Comment 22

16 years ago
any comment about the review, roy?

Updated

16 years ago
Blocks: 107067

Updated

16 years ago
Keywords: nsbranch-

Comment 23

16 years ago
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

16 years ago
ftang: it works fine on my WinXP. As your document stays we need
ExitProcess() to end the process created by CreateProcess()

Comment 25

16 years ago
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

16 years ago
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.
Attachment #55143 - Flags: needs-work+
(Reporter)

Updated

16 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 27

16 years ago
move to 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
(Reporter)

Comment 28

16 years ago
mass move to m9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Updated

15 years ago
No longer blocks: 107067
Whiteboard: [correctness], PDT- → [correctness]
(Reporter)

Comment 29

15 years ago
mark this as future for now.
Target Milestone: mozilla0.9.9 → Future
(Reporter)

Updated

15 years ago
No longer blocks: 104056
(Reporter)

Comment 30

12 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → WONTFIX

Comment 31

12 years ago
Mass Re-open of Frank Tangs Won't fix debacle. Spam is his responsibility not my own
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---

Comment 32

12 years ago
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
Assignee: ftang → nobody
Status: REOPENED → NEW
Depends on: 352049
bug 352049 killed the font download dialog, marking all bugs about it wontfix.
Status: NEW → RESOLVED
Last Resolved: 12 years ago11 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.