Clean up nsLocaleService::nsLocaleService string fu

RESOLVED DUPLICATE of bug 195093

Status

()

RESOLVED DUPLICATE of bug 195093
18 years ago
15 years ago

People

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

Tracking

({helpwanted, intl})

Trunk
Future
helpwanted, intl
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

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 :-)
(Reporter)

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) 
from
  lang = "en-US";
to
  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.

     nsCRT::free(lc_temp);

should be

     UniFreeMem(lc_temp);

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

Comment 6

18 years ago
bstell:

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 :-)

mkaply:

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.
(Reporter)

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.
(Reporter)

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
r=mkaply
(Reporter)

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.
(Reporter)

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

Comment 15

18 years ago
This can wait till 0.9.2.
(Reporter)

Comment 16

18 years ago
Mass move to jaggernaut@netscape.com
Assignee: jag → jaggernaut

Comment 17

18 years ago
->0.9.3
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?
(Assignee)

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
(Assignee)

Comment 21

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

Comment 22

17 years ago
->0.9.5
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
(Assignee)

Comment 24

17 years ago
->0.9.8
Target Milestone: mozilla0.9.6 → mozilla0.9.8
(Assignee)

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
appropriate.
QA Contact: andreasb → bobj

Comment 27

15 years ago
retargeting
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 ***
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.