254298 - nsPlainTextSerializer.cpp should not use AssignWithConversion

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

content/base/src/nsPlainTextSerializer.cpp should not use AssignWithConversion

once bug 252950 is fixed, m1b will be ascii, and this file can use AssignASCII.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 570634 [details] [diff] [review]
fix


>   else {
>-    textstr.AssignWithConversion(frag->Get1b()+aStartOffset, length);
>+    // AssignASCII is for 7-bit character only, so don't use it
>+    const char *data = frag->Get1b();
>+    CopyASCIItoUTF16(Substring(data + aStartOffset, data + endoffset), textstr);
>   }

Interesting. Would it be possible to get a test which would fail if AssignASCII was used?

Comment 3

[Makoto Kato [:m_kato]]

(In reply to Olli Pettay [:smaug] from comment #2)
> Comment on attachment 570634 [details] [diff] [review] [diff] [details] [review]
> fix
> 
> 
> >   else {
> >-    textstr.AssignWithConversion(frag->Get1b()+aStartOffset, length);
> >+    // AssignASCII is for 7-bit character only, so don't use it
> >+    const char *data = frag->Get1b();
> >+    CopyASCIItoUTF16(Substring(data + aStartOffset, data + endoffset), textstr);
> >   }
> 
> Interesting. Would it be possible to get a test which would fail if
> AssignASCII was used?

Several tests are failure if using AssignASCII.

https://tbpl.mozilla.org/php/getParsedLog.php?id=7117846&tree=Try
https://tbpl.mozilla.org/php/getParsedLog.php?id=7117826&tree=Try
https://tbpl.mozilla.org/php/getParsedLog.php?id=7117897&tree=Try

Although AssignASCII uses char_traits::copyASCII(), it works on 7-bit character only.  If using 8-bit character, it will be different result due to cast.

http://mxr.mozilla.org/mozilla-central/source/xpcom/string/public/nsCharTraits.h#222

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 570634 [details] [diff] [review]
fix

Ok, thanks.

Comment 5

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89649127982e

Comment 6

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/89649127982e