Closed Bug 254298 Opened 16 years ago Closed 8 years ago

nsPlainTextSerializer.cpp should not use AssignWithConversion

Categories

(Core :: DOM: Serializers, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: Biesinger, Assigned: m_kato)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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

once bug 252950 is fixed, m1b will be ascii, and this file can use AssignASCII.
Assignee: dom-to-text → nobody
QA Contact: dom-to-text
Blocks: 113234
Attached patch fixSplinter Review
Assignee: nobody → m_kato
Attachment #570634 - Flags: review?(Olli.Pettay)
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?
(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 on attachment 570634 [details] [diff] [review]
fix

Ok, thanks.
Attachment #570634 - Flags: review?(Olli.Pettay) → review+
https://hg.mozilla.org/mozilla-central/rev/89649127982e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.