Last Comment Bug 254298 - nsPlainTextSerializer.cpp should not use AssignWithConversion
: nsPlainTextSerializer.cpp should not use AssignWithConversion
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Serializers (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla10
Assigned To: Makoto Kato [:m_kato]
:
:
Mentors:
http://lxr.mozilla.org/seamonkey/sour...
Depends on: 252950
Blocks: 113234
  Show dependency treegraph
 
Reported: 2004-08-04 09:53 PDT by Christian :Biesinger (don't email me, ping me on IRC)
Modified: 2014-04-26 02:23 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.65 KB, patch)
2011-10-31 03:38 PDT, Makoto Kato [:m_kato]
bugs: review+
Details | Diff | Splinter Review

Description Christian :Biesinger (don't email me, ping me on IRC) 2004-08-04 09:53:31 PDT
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] 2011-10-31 03:38:44 PDT
Created attachment 570634 [details] [diff] [review]
fix
Comment 2 Olli Pettay [:smaug] (reviewing overload) 2011-10-31 04:37:26 PDT
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] 2011-10-31 20:12:34 PDT
(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] (reviewing overload) 2011-11-01 04:25:02 PDT
Comment on attachment 570634 [details] [diff] [review]
fix

Ok, thanks.
Comment 6 Matt Brubeck (:mbrubeck) 2011-11-04 11:37:30 PDT
https://hg.mozilla.org/mozilla-central/rev/89649127982e

Note You need to log in before you can comment on or make changes to this bug.