Clean up nsLocaleService::nsLocaleService string fu




18 years ago
15 years ago


(Reporter: jag-mozbugs, Assigned: jag-mozilla)


({helpwanted, intl})

helpwanted, intl

Firefox Tracking Flags

(Not tracked)



(2 attachments)



18 years ago
For bug 64016 I ran into some ugly string code in
nsLocaleService::nsLocaleService. I fixed it, but didn't want to land it as part
of that bug (purity reasons), so here's a bug I can attach my fix to :-)

Comment 1

18 years ago
Created attachment 26613 [details] [diff] [review]
[patch] clean-up

Comment 2

18 years ago
Thanks for the patch, let me reassign to you since you have a patch.
Cc to bstell, mkaply.
Brian, please review first half of the patch.
Mike, please reivew os/2 part of the change.
Assignee: nhotta → disttsc

Comment 3

18 years ago
adding keyword intl and nsbeta1.
Keywords: intl, nsbeta1

Comment 4

18 years ago
please change the first locale fallback string (about line 237 in the source) 
  lang = "en-US";
  lang = "en_US";

which is the platform version of that locale.

Other than that it looks good.
For the unix half: r=bstell

Comment 5

18 years ago
lc_temp is still allocated by the UniAPI, so it should use UniFreeMem. This 
looks like it was a bug in the original code.


should be


Also, should OS/2 be Releasing the converter in the NS_FAILED case, the same was 
as Unix?

Comment 6

18 years ago

Whoops, nice catch. So this is what the original code did:

-                    nsCAutoString langcstr("en-US");
-                    platformLocale.AssignWithConversion("en_US");

I missed the - vs _, I'll go fix my code :-)


Ah, okay, I understand now that different allocators were used. Hmmm, so much
for merging that code. Okay, I'll fix that. About releasing the converter in the
failed case, no, OS/2 is the only one using nsCOMPtrs currently, so it gets
automatically released when it goes out of scope.

New version coming up.

Comment 7

18 years ago
mkaply: btw. in the original code nsCRT::free was used in case NS_FAILED(result)
and UniFreeMem otherwise. I guess the nsCRT::free there was incorrect.

Comment 8

18 years ago
Created attachment 26694 [details] [diff] [review]
[patch] fixed incorrect changes, use nsCOMPtr in XP_UNIX

Comment 9

18 years ago

Comment 10

18 years ago
bstell: could you r= the new patch?

Who should I ask for r= of the Windows part?

Comment 11

18 years ago
jag - This looks like it is pretty close. can we get this by M0.9.1. If not, pls 
assign M0.9.2 milestone.

Comment 12

18 years ago
Tentatively setting to 0.9.2
Target Milestone: --- → mozilla0.9.2

Comment 13

18 years ago
marking as nsbeta1-.
Keywords: nsbeta1 → nsbeta1-

Comment 14

18 years ago
It is very close to the beta release point.
Can we wait till after 0.9.1 ?

Comment 15

18 years ago
This can wait till 0.9.2.

Comment 16

18 years ago
Mass move to
Assignee: jag → jaggernaut

Comment 17

18 years ago
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Comment 18

17 years ago
Do we have a patch. Should we move this to TM0.9.4, and schedule it to be 
checked-in to the trunk?

Comment 19

17 years ago
That patch has bit rotted. I will see if I get around to updating it for 0.9.3
or 0.9.4.

Comment 20

17 years ago
Thanks Jag

Comment 21

17 years ago
-> 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Comment 22

17 years ago
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 23

17 years ago
Mass-moving lower-priority 0.9.5 bugs off to 0.9.6 to make way for remaining
0.9.4/eMojo bugs, and MachV planning, performance and feature work. If you
disagree with any of these targets, please let me know.
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Comment 24

17 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.8

Comment 25

17 years ago
If someone wants to take this patch and update it to the trunk, go for it.
-> 1.1
Keywords: helpwanted
Target Milestone: mozilla0.9.8 → mozilla1.1

Comment 26

16 years ago
Changing QA contact to bobj for now. Bob, please re-assign further as you see is
QA Contact: andreasb → bobj

Comment 27

15 years ago
Target Milestone: mozilla1.1alpha → Future

Comment 28

15 years ago
bug 195093 is a superset of this bug. I just compared my patch for bug 195093
with attachment 26694 [details] [diff] [review] and they're more or less similar (for nsLocalService.cpp.
my patch cleaned up other parts of intl/locale as well) although the patch here
is a bit more streamlined in one or two places.  I'm marking this as a dupe of
bug 195093 because I've got a patch (against the current cvs) that can go in as
soon as 1.4beta cycle begins (hopefully in a couple of days).

*** This bug has been marked as a duplicate of 195093 ***
Last Resolved: 15 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.