Status

SeaMonkey
General
P3
normal
RESOLVED INVALID
19 years ago
14 years ago

People

(Reporter: dougt, Assigned: Roy Yokoyama)

Tracking

({memory-footprint, perf})

Trunk
mozilla0.9
memory-footprint, perf

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(26 attachments)

2.47 KB, text/plain
Details
23.81 KB, patch
Details | Diff | Splinter Review
30.35 KB, patch
Details | Diff | Splinter Review
20.16 KB, patch
Details | Diff | Splinter Review
35.16 KB, patch
Details | Diff | Splinter Review
249.13 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.13 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
1.25 KB, patch
Details | Diff | Splinter Review
1.40 KB, patch
Details | Diff | Splinter Review
34.07 KB, patch
Details | Diff | Splinter Review
249.13 KB, patch
Details | Diff | Splinter Review
32.83 KB, patch
Details | Diff | Splinter Review
25.58 KB, text/html
Details
(Reporter)

Description

19 years ago
The following file copied nsIModule.  Please use the macro NS_IMPL_NSGETMODULE
instead.

D:\cmonkey\mozilla\intl\chardet\src\nsCharDetModule.cpp(149):
D:\cmonkey\mozilla\intl\compatibility\src\nsI18nCompatibility.cpp(177):
D:\cmonkey\mozilla\intl\locale\src\unix\nsLocaleModule.cpp(129):
D:\cmonkey\mozilla\intl\lwbrk\src\nsLWBrkModule.cpp(129):
D:\cmonkey\mozilla\intl\strres\src\nsStringBundle.cpp(506):
D:\cmonkey\mozilla\intl\uconv\src\nsUConvModule.cpp(101):
D:\cmonkey\mozilla\intl\uconv\ucvcn\nsUCvCnModule.cpp(313):
D:\cmonkey\mozilla\intl\uconv\ucvja\nsUCvJaModule.cpp(347):
D:\cmonkey\mozilla\intl\uconv\ucvko\nsUCvKoModule.cpp(281):
D:\cmonkey\mozilla\intl\uconv\ucvlatin\nsUCvLatinModule.cpp(976):
D:\cmonkey\mozilla\intl\uconv\ucvtw\nsUCvTwModule.cpp(271):
D:\cmonkey\mozilla\intl\uconv\ucvtw2\nsUCvTw2Module.cpp(361):
D:\cmonkey\mozilla\intl\unicharutil\src\nsUcharUtilModule.cpp(150):

Comment 1

19 years ago
dp, please provide details about this bug. We have no idea what you talk about.

Comment 2

19 years ago
Sure frank. Look at xpcom/sample/nsSampleModule.cpp

We have embarked on a macro version of factory and module. A lot of code for
these can go away. Converting to the above template will reduce our dll size.

Dougt has converted a lot of them. I did a few before the holidays. The ones
that dougt didnt convert have bugs I think. (BTW thanks doug)

Updated

19 years ago
Status: NEW → ASSIGNED

Updated

19 years ago
Target Milestone: M13

Updated

19 years ago
Depends on: 23350

Comment 3

19 years ago
nsUcharUtilModule.cpp fix in my tree and send to dp for code review.
nsUcv*Module.cpp, nsCharDetModule.cpp depend on 23350
I will work on nsI18nCompatibility.cpp, nsLocaleModule.cpp,
nsStringBundle.cpp and nsUConvModule.cpp first.

Comment 4

19 years ago
nsI18nCompatibility.cpp is fix in my tree. Send to dp and nhotta for review.

Comment 5

19 years ago
Noooo!!!! Please don't do this indiscriminately!!! In some of the cases, we are
using our own Module object some reasons: at initialization time we
are writing stuff into the registry, we delete that stuff at the end and we
create components in a certain way. The places where we are doing that
are: uconv and the ucv* DLLs. So please don't change those.

Thanks,
Cata

Comment 6

19 years ago
Created attachment 4105 [details]
nsUConvModule.cpp based on r1.2

Comment 7

19 years ago
archive the r1.2 based nsUConvModule.cpp since cata ask me to hold it.

Updated

19 years ago
Target Milestone: M13 → M14

Comment 8

19 years ago
Cannot fix all the module by m13. Move it to M14.

Comment 9

19 years ago
reassign to jbetak
Assignee: ftang → jbetak
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED

Comment 10

19 years ago
ftang,
What is the status on this?  Which modules have we updated and checked-in?
Is this bug a Beta1 stopper?
(Reporter)

Comment 11

19 years ago
This is not a beta1 stopper.  there are other more important things to do.  
(IMO)

Comment 12

19 years ago
Move this to M15
Target Milestone: M14 → M15

Comment 13

19 years ago
jbetak- I suggest you do nsUCvCnModule.cpp first.

Comment 14

19 years ago
*** Bug 32792 has been marked as a duplicate of this bug. ***

Updated

19 years ago
Target Milestone: M15 → ---
Target Milestone: --- → M18

Comment 15

18 years ago
*spam* changing qa contact from nobody@mozilla.org to me (BlakeR1234@aol.com) 
on 121 open or resolved (but not verified) bugs.  sorry for the spam everybody, 
but most of these bugs would just remain dormant and not checked by QA 
otherwise.  I'm not sure how so many bugs have nobody as their QA contact, but 
I suspect this is the fault of some sort of bugzilla corruption that happened 
at some point (most of these bugs are in the 20000-26000 range, and I don't see 
where in the activity log that QA contact explicitly changed to 
nobody@mozilla.org)

Anyways, sorry again for spam.  If you really get annoyed, I'm usually 
available in #mozilla on IRC for torture.
QA Contact: nobody → BlakeR1234
reassigning to ftang for resource reallocation
Assignee: jbetak → ftang
Status: ASSIGNED → NEW

Comment 17

18 years ago
mark nsbeta3 and perf
Status: NEW → ASSIGNED
Keywords: nsbeta3, perf

Comment 18

18 years ago
nsbeta3- per i18n bug meeting.
dp- what is your opinion. we are open to re-evaluate this bug for nsbeta3 if you 
think it is very important.
Whiteboard: [nsbeta3-]

Comment 19

18 years ago
If this will help performance and code footprint (the main goals of beta3), why 
nsbeta3- ?
Keywords: footprint

Comment 20

18 years ago
yea, I'm asking for reconsideration.
Whiteboard: [nsbeta3-]

Comment 21

18 years ago
The following module are done
nsI18nCompatibility
ucvtw
ucvja
nsLWBrkModule
nsStringBundle
nsUcharUtilModule

The following are not done
uconv
ucvtw2
ucvko
ucvlatin
ucvcn
ucvibm
nsCharDetModule
nslocale
nsI18nCompatibility

dp said he don't think this is important.
(Reporter)

Comment 22

18 years ago
dp - why now does this not matter.  Your comments on 01-03:

We have embarked on a macro version of factory and module. A lot of code for
these can go away. Converting to the above template will reduce our dll size.


I think now more than ever, DLL size is important.  

Comment 23

18 years ago
Marking as nsbeta3- per I18N Bug Triage.

DP told Ftang that this issue is not of high importance.
Whiteboard: [nsbeta3-]
(Reporter)

Comment 24

18 years ago
I, the reporter, and DP inital comment disagree with this.  
Whiteboard: [nsbeta3-]

Comment 25

18 years ago
nsbeta3- per i18n bug meeting. I don't think we have enough time to fix and 
test this. 
Whiteboard: [nsbeta3-]

Updated

18 years ago
QA Contact: blakeross → doronr

Comment 26

18 years ago
reassign to yokoyama

plese try to fix
ucvtw2
ucvko
ucvcn

first. and then
ucvlatin
ucvibm
Assignee: ftang → yokoyama
Status: ASSIGNED → NEW
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 27

18 years ago
Created attachment 20373 [details] [diff] [review]
ucvtw2 converted module
(Assignee)

Comment 28

18 years ago
Frank: Some of the charsets (eg. UnicodeToISO2022KR) in nsUCvKoCID.h are not in
nsUCvKoModule.cpp.  Is this correct?
(Assignee)

Comment 29

18 years ago
Created attachment 20381 [details] [diff] [review]
Ooops, compiler error.  Here is the update.  Disregard the previous attachement (ucvtw2)
(Assignee)

Comment 30

18 years ago
Created attachment 20383 [details] [diff] [review]
ucvko converted module
(Assignee)

Comment 31

18 years ago
Cata: How can I verify the conversion? I can view a Korean site with my patch,
but I don't think it's good enough.  Do you have a test plan for all the encodings?

adding cata@netscape.com to cc

Comment 32

18 years ago
nice work! r=valeski on the 12/08/00 16:42 and 12/08/00 17:32 patches.
(Assignee)

Comment 33

18 years ago
Created attachment 20531 [details] [diff] [review]
ucvcn converted module.  This was very tricky.

Comment 34

18 years ago
nice work again. r=valeski on 12/12/00 10:30 patch

Comment 35

18 years ago
It's ok, testing with a page using the respective encoding is the best test.
(Assignee)

Comment 36

18 years ago
Cata: Do you have a list of URLs for respective encodings?  I checked against
home.netscape.com/ja, /ko etc and Yahoo.

Comment 37

18 years ago
Nope. You should ask Kat, I'm sure he has.
(Assignee)

Comment 38

18 years ago
cc'ing: momoi-san
momoi-san: Do you have a list of URLs to do a quick testing on Char Encodings?

Comment 39

18 years ago
<offtopic>
Changing keywords, clobbering milestone. yokoyama could you or a manager select 
a current target milestone (mozilla0.8 currently is the next milestone, other 
candidates include mozilla0.9 and mozilla1.0).

fwiw, these days we're moving towards using cvsblame to show authorship instead 
of inline ownership marks.

+ * The Original Code is Mozilla Communicator client code.
Do we say this elsewhere?
</offtopic>
Keywords: nsbeta3 → approval, mozilla1.0, patch
Whiteboard: [nsbeta3-]
Target Milestone: M18 → ---
(Assignee)

Comment 40

18 years ago
Created attachment 20923 [details] [diff] [review]
ucvlatin converted module (16 more files to add)
(Assignee)

Comment 41

18 years ago
Created attachment 20924 [details] [diff] [review]
ucvlatin converted module (1/16)
(Assignee)

Comment 42

18 years ago
Created attachment 20925 [details] [diff] [review]
ucvlatin converted module (2/16)
(Assignee)

Comment 43

18 years ago
Created attachment 20926 [details] [diff] [review]
ucvlatin converted module (3/16)
(Assignee)

Comment 44

18 years ago
Created attachment 20927 [details] [diff] [review]
ucvlatin converted module (4/16)
(Assignee)

Comment 45

18 years ago
Created attachment 20928 [details] [diff] [review]
ucvlatin converted module (5/16)
(Assignee)

Comment 46

18 years ago
Created attachment 20929 [details] [diff] [review]
ucvlatin converted module (6/16)
(Assignee)

Comment 47

18 years ago
Created attachment 20930 [details] [diff] [review]
ucvlatin converted module (7/16)
(Assignee)

Comment 48

18 years ago
Created attachment 20931 [details] [diff] [review]
ucvlatin converted module (8/16)
(Assignee)

Comment 49

18 years ago
Created attachment 20932 [details] [diff] [review]
ucvlatin converted module (9/16)
(Assignee)

Comment 50

18 years ago
Created attachment 20933 [details] [diff] [review]
ucvlatin converted module (10/16)
(Assignee)

Comment 51

18 years ago
Created attachment 20934 [details] [diff] [review]
ucvlatin converted module (11/16)
(Assignee)

Comment 52

18 years ago
Created attachment 20935 [details] [diff] [review]
ucvlatin converted module (12/16)
(Assignee)

Comment 53

18 years ago
Created attachment 20936 [details] [diff] [review]
ucvlatin converted module (13/16)
(Assignee)

Comment 54

18 years ago
Created attachment 20937 [details] [diff] [review]
ucvlatin converted module (14/16)
(Assignee)

Comment 55

18 years ago
Created attachment 20938 [details] [diff] [review]
ucvlatin converted module (15/16)
(Assignee)

Comment 56

18 years ago
Created attachment 20939 [details] [diff] [review]
ucvlatin converted module (16/16) - This is too much. I hope I didn't make any mistake:)
(Assignee)

Comment 57

18 years ago
Created attachment 20989 [details] [diff] [review]
ucvibm converted module
(Assignee)

Comment 58

18 years ago
Could someone review the code?
- ucvtw2 : reviewed by Jud (2000-12-11 10:34)
- ucvko : reviewed by Jud (2000-12-11 10:34)
- ucvcn : reviewed by Jud (2000-12-12 10:38)
- cvlatin : need a review
- ucvibm : need a review

Momoi-san:  do we have URLs to test respective encodings?  

Comment 59

18 years ago
Some internal sites which contain charset tests:

http://slip/i18n_client/browser/fonts/multibyte_tests/
http://babel/tests/Browser/charset/

You will find EUC-TW CNS planes 1 & 2 here:

http://slip/i18n_client/browser/fonts/multibyte_tests/euc-tw/euc-tw.html

I don't think we have too many IBM code pages for testing
internally. One that comes to my mind is Cyrillic 866
at:

http://babel/tests/Browser/charset/charset_test.html
(Assignee)

Comment 60

18 years ago
Created attachment 21215 [details] [diff] [review]
Correction for (12/18/00 17:47) ucvlatin converted module
(Assignee)

Comment 61

18 years ago
cc: ftang@netscape.com
(Assignee)

Comment 62

18 years ago
Frank: as per our code review, please /r for the new ucvlatin as well as ucvibm.
Thanks
(Assignee)

Comment 63

18 years ago
/sr by ftang.  This is a big change. I'll notify iQA by email and keep my eyes
open for any new issues.

Comment 64

18 years ago
What about ucvmath!?
(Assignee)

Comment 65

18 years ago
Created attachment 23198 [details] [diff] [review]
ucvmath converted module
(Assignee)

Comment 66

18 years ago
rbs@maths.uq.edu.au < can you review the code for me?

Comment 67

18 years ago
To test the rendering International fonts, please go to
http://babel/tests/dogfood/templates/top_intl_urls.htm

Comment 68

18 years ago
I didn't applied the patch. But I have scanned it and couldn't spot at anything
suspicious. r=rbs.

Comment 69

18 years ago
Created attachment 23269 [details]
Test urls
(Assignee)

Comment 70

18 years ago
Updating the target milestone.
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 71

18 years ago
ftang: can you /sr= for the ucvmath (01/22/01 15:13)?

Comment 72

18 years ago
for ucvmath sr=ftang
(Assignee)

Comment 73

18 years ago
All the modules are checked in. :)  Please verify.  
Thanks
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 74

18 years ago
I need to convert uconv.dll and others as well.  Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

18 years ago
Target Milestone: mozilla0.8 → mozilla0.9
(Assignee)

Comment 75

18 years ago
We have little more module to do:
nsUconvModule
nsChardetModule
nsLocaleModule
nsLwbrkModule
nsUCharUtilModule
(Assignee)

Comment 76

18 years ago
Revising the list of components left:
nsUconvModule - need to use NS_GETMODULE
nsChardetModule  (67576 will resolve this)
nsLocaleModule - need to use NS_GETMODULE
nsLwbrkModule - already done
nsUCharUtilModule - already done

I am going to file new bugs for nsLocaleModule and nsUConvModule.  I hate to
attach patches to this bug after seeing long list of patches attached. :)
(Assignee)

Comment 77

18 years ago
I filed 2 new bugs:
74438 nor -- All yokoyama@netscape.com NEW nsUconvModule - need to use NS_GETMODULE
74439 nor -- All yokoyama@netscape.com NEW nsLocaleModule - need to use NS_GETMODULE

doronr@naboonline.com> please close this bug.

Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → INVALID
Product: Browser → Seamonkey
Attachment #20923 - Attachment description: ucvlatin converted module (16 more files to add:nsISO88596EToUnicode.cpp, nsISO88596EToUnicode.h, nsISO88596IToUnicode.cpp, nsISO88596IToUnicode.h, nsISO88598EToUnicode.cpp,nsISO88598EToUnicode.h, nsISO88598IToUnicode.cpp, nsISO88598IToUnicode.h, nsUnicod → ucvlatin converted module (16 more files to add)
You need to log in before you can comment on or make changes to this bug.