87677 - implement CopyUTF8toUCS2() and CopyUCS2toUTF8()

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Chris Waterson]

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.

Comment 1

[Peter Van der Beken [:peterv]]

Any news on this?

Comment 2

[Scott Collins]

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

Comment 3

[Alec Flett]

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.

Comment 4

[Alec Flett]

I lost all the work I started here in a hard drive crash yesterday. :(
starting over...

Comment 5

[Johnny Stenback (:jst)]

Attachment: How about just this for now?

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 6

[Alec Flett]

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 7

[Johnny Stenback (:jst)]

Comment on attachment 121717 [details] [diff] [review]
How about just this for now?

Requesting jag's review, and marking alecf's sr.

Comment 8

[jag (Peter Annema)]

Comment on attachment 121717 [details] [diff] [review]
How about just this for now?

r=jag

Comment 9

[Axel Hecht]

If one moved ConvertUTF8toUCS2 and CalculateUTF8Length from nsString2 to 
nsReadableString, this should be fairly easy. From a short glimpse at the code.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Re comment 9: That's done in my patch on bug 131293, which is awaiting review.

Comment 11

[jag (Peter Annema)]

Once bug 206682 is fixed, you might be able to use that for better
implementations of these functions.

Comment 12

[Johnny Stenback (:jst)]

Fix checked in. Should we leave this open to make these not suck, or should we
do that in another bug?

Comment 13

[Jungshik Shin]

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. 

Comment 14

[Alec Flett]

I'm fine with that... if you submit a patch here to rename the ones that just
went in, I'll review it.

Comment 15

[Johnny Stenback (:jst)]

Attachment: Make those functions suck less.

Comment 16

[jag (Peter Annema)]

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.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Or maybe we do use them -- I haven't really looked at the sliding strings in
the parser.)

Comment 19

[jag (Peter Annema)]

Comment on attachment 125551 [details] [diff] [review]
Make those functions suck less.

r=jag

Comment 20

[Johnny Stenback (:jst)]

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...

Comment 21

[Alec Flett]

well, we should open a new bug for converting some performance-intensive
consumers, like nsIAtom

Comment 22

[jag (Peter Annema)]

New bug then?

Comment 23

[Alec Flett]

so the new bug is bug 209699 - feel free to attach patches there, I'll help review.