Move show_punycode pref into nsStandardURL

RESOLVED FIXED in mozilla1.8beta2

Status

()

P1
normal
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: darin.moz, Assigned: darin.moz)

Tracking

Trunk
mozilla1.8beta2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

14 years ago
Move show_punycode pref into nsStandardURL

Per https://bugzilla.mozilla.org/show_bug.cgi?id=283992#c14, it would be better
to move pref checking into nsStandardURL so that the core IDN conversion
routines can still be used by other consumers.
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
(Assignee)

Comment 1

14 years ago
Created attachment 180014 [details] [diff] [review]
v1 patch
Attachment #180014 - Flags: review?(jshin)
(Assignee)

Updated

14 years ago
Depends on: 283992
+    static nsIIDNService               *gIDN;

shouldn't that be sIDN, as it's a class static, not a global? (and the other
static members too...)
(Assignee)

Comment 3

14 years ago
(In reply to comment #2)
> +    static nsIIDNService               *gIDN;
> 
> shouldn't that be sIDN, as it's a class static, not a global? (and the other
> static members too...)

"pick a style, any style, but be consistent"  ... i chose to be consistent with
the existing code rather than bloat the patch.
(Assignee)

Updated

14 years ago
Attachment #180014 - Flags: superreview?(dbaron)
Attachment #180014 - Flags: review?(jshin)
Attachment #180014 - Flags: review?(cbiesinger)
Comment on attachment 180014 [details] [diff] [review]
v1 patch

sr=dbaron (I even went back and compared to bug 282270)
Attachment #180014 - Flags: superreview?(dbaron) → superreview+
Comment on attachment 180014 [details] [diff] [review]
v1 patch

netwerk/base/src/nsStandardURL.cpp
	     mHostA = ToNewCString(result);

hm... I wonder why this is not an nsCString or something...
Attachment #180014 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 6

14 years ago
> netwerk/base/src/nsStandardURL.cpp
> 	     mHostA = ToNewCString(result);
> 
> hm... I wonder why this is not an nsCString or something...

Because we almost never allocate it.  I chose to reserve 4 bytes instead of 16
for this member variable.  I suppose now that nsCString supports buffer sharing,
that we could benefit from allocating a new nsCString object.  Hmm :-/
(Assignee)

Comment 7

14 years ago
Comment on attachment 180014 [details] [diff] [review]
v1 patch

This could slip until 1.8b3 if we are getting tight with approvals.
Attachment #180014 - Flags: approval1.8b2?

Comment 8

14 years ago
Comment on attachment 180014 [details] [diff] [review]
v1 patch

a=asa
Attachment #180014 - Flags: approval1.8b2? → approval1.8b2+
(Assignee)

Comment 9

14 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.