44074 - Use nsAReadableString instead of nsString in nsHTMLToTXTSinkStream::Write

Status: VERIFIED FIXED

(Core :: DOM: Serializers, defect, P3)

Description

[Daniel Bratell]

scc pointed out that we could probably be more efficient if we used 
nsAReadableString instead of nsString as argument to the Write method in 
nsHTMLToTXTSinkStream::Write. 

From mail:
while reviewing some code for Ben Bucksch, I saw that your |Write|
method with which Ben is interacting (see

   <http://bugzilla.mozilla.org/show_bug.cgi?id=32336>

) could, perhaps, allow more efficient patterns in callers if it took
a |const nsAReadableString&| rather than directly requiring any kind
of reference to the implementation type |nsString|.  This would allow
people to call it on a literal string without copying and conversion.

Comment 1

[Daniel Bratell]

I started looking at this, but changing one place requires changes somewhere 
else which in turn requires changes in some other place. 

What I more specifically was stuck on was the unicode converter. The unicode 
converter wants char and PR_Unichar pointers and we can't get them from 
nsAReadableString. Assigning the nsAReadableString to a nsString and then 
calling GetUnicode on that nsString kind of beats the whole purpose of this 
exercise.

I could do with methods 
Convert(nsAReadableString& src, nsCString& destination)
and
Finish(nsCString& destination) 

in the nsIUnicodeConverter. I filed bug 46165 on that.

Comment 2

[Scott Collins]

A lot more places are allowing |nsAReadable[C]Strings| now with vidur and jst's 
recent DOM changes.  Probably suitable candidates for the conversion functions 
you mention in bug #46165 now exist.  This should help.

Comment 3

[Daniel Bratell]

Vidur's latest changes to the parser fixed this. 

Comment 4

[sujay]

Daniell, please verify....mark VERIFIED-FIXED after doing so...thanks!

Comment 5

[Daniel Bratell]

Well, I looked once again and it's still fixed. Marking verified.