Closed Bug 87677 Opened 23 years ago Closed 21 years ago

implement CopyUTF8toUCS2() and CopyUCS2toUTF8()

Categories

(Core :: XPCOM, defect, P1)

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.5alpha

People

(Reporter: waterson, Assigned: alecf)

References

Details

Attachments

(2 files)

We need analogues to NS_COM void CopyUCS2toASCII( const nsAString& aSource, nsACString& aDest ); NS_COM void CopyASCIItoUCS2( const nsACString& aSource, nsAString& aDest ); that handle UTF-8 encoded strings.
Any news on this?
giving up ancient string bugs to the new string owner. jag, you'll want to sort through these and see which ones still apply and go with or against the direction in which you intend strings evolve
Assignee: scc → jaggernaut
Ok, I've started work here. its going to be a bitch.. mostly moving code out of nsString/nsCString, and converting it over to iterators instead of raw character pointers I need this to make bug 195262 performant.
Assignee: jaggernaut → alecf
Target Milestone: --- → mozilla1.4alpha
I lost all the work I started here in a hard drive crash yesterday. :( starting over...
Status: NEW → ASSIGNED
Target Milestone: mozilla1.4alpha → mozilla1.4beta
Priority: -- → P1
Target Milestone: mozilla1.4beta → mozilla1.5alpha
Attached patch How about just this for now? (deleted) — Splinter Review
This is silly, but it would let us start using these methods, even if they use code that lives in the wrong place. I also added AppendXtoY (for UTF8 and UCS2) since those are useful too in some cases.
Comment on attachment 121717 [details] [diff] [review] How about just this for now? oh! yes this is a great idea. Instead of fixing up eight million callsites. sr=alecf
Comment on attachment 121717 [details] [diff] [review] How about just this for now? Requesting jag's review, and marking alecf's sr.
Attachment #121717 - Flags: superreview+
Attachment #121717 - Flags: review?(jaggernaut)
Comment on attachment 121717 [details] [diff] [review] How about just this for now? r=jag
Attachment #121717 - Flags: review?(jaggernaut) → review+
If one moved ConvertUTF8toUCS2 and CalculateUTF8Length from nsString2 to nsReadableString, this should be fairly easy. From a short glimpse at the code.
Re comment 9: That's done in my patch on bug 131293, which is awaiting review.
Once bug 206682 is fixed, you might be able to use that for better implementations of these functions.
Fix checked in. Should we leave this open to make these not suck, or should we do that in another bug?
Can we make a trivial change? UCS2 in function names should be UTF16 because that's what's converted to and from. See bug 183156. Because these two are very new, nobody has used them (I'm gonna use it in my patch for bug 162765) and they're the perfect target for the 'rectification'. I've just uploaded the patch there. If further work is gonna be done on this front, it'd be nice to keep in mind that we need to use UTF16 instead of UCS2 in any new functions.
I'm fine with that... if you submit a patch here to rename the ones that just went in, I'll review it.
Attachment #125551 - Flags: superreview?(alecf)
Attachment #125551 - Flags: review?(jaggernaut)
Comment on attachment 125551 [details] [diff] [review] Make those functions suck less. alecf: no offense, but I'd like dbaron to take a look at this.
Attachment #125551 - Flags: superreview?(alecf) → superreview?(dbaron)
Comment on attachment 125551 [details] [diff] [review] Make those functions suck less. Looks fine to me except that you don't need the |.write_terminator()| (in both places) since you used |SetLength|. (See bug 114140 for some interesting history. Also, I'm not sure if the string APIs should really support multi-fragment writable strings, since we don't use them, but they do...)
Attachment #125551 - Flags: superreview?(dbaron) → superreview+
(Or maybe we do use them -- I haven't really looked at the sliding strings in the parser.)
Comment on attachment 125551 [details] [diff] [review] Make those functions suck less. r=jag
Attachment #125551 - Flags: review?(jaggernaut) → review+
Fix checked in. Anything more to do here? I don't think so, marking FIXED, reopen if there's more to do that I'm unaware of...
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
well, we should open a new bug for converting some performance-intensive consumers, like nsIAtom
Blocks: 209699
New bug then?
so the new bug is bug 209699 - feel free to attach patches there, I'll help review.
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: