40350 - nsPlainTextSerializer not work if input is NCR

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P3)

Description

[nhottanscp]

1) Create a HTML file which contains NCR (AÁ).
2) Open that in browser, it shows "AÁ" (capital 'A' and AACUTE).
3) In messenger account set up, use this HTML as a signature file also uncheck 
to compose HTML mail.
4) Create a new message. A plain text mail shows up and the signature turns to 
square boxes (on WinNT).

In the code messenger uses nsIHTMLToTXTSinkStream to convert from HTML to plain 
text. When the input HTML contain NCR, NCR turned to 0xFFFF after the 
conversion.

Comment 1

[nhottanscp]

This bug was separated from 34373.

Comment 2

[Akkana Peck]

This is probably related to bug 13401, that the nsHTMLToTXTSinkStream needs to
be converted to use nsISaveAsCharset just like the nsHTMLContentSinkStream
already was.

Marking M19 (13401 is M20 because no one seemed to care about it), but if this
is important, let me know and I'll do it sooner.  It shouldn't be difficult.

Comment 3

[nhottanscp]

nsISaveAsCharset generates NCR but this bug is about interpreting NCR, so not 
sure if nsISaveAsCharset should be used.

Comment 4

[Akkana Peck]

*** Bug 13401 has been marked as a duplicate of this bug. ***

Comment 5

[Akkana Peck]

Per beppe, marking future.  Is this blocking anyone?

Comment 6

[nhottanscp]

This was a problem since HTML composer generated NCR for Japanese HTML.
I think currently that does not happen (text saved as a target charset instead 
of NCR).

Comment 7

[rubydoo123]

adding help wanted keyword

Comment 8

[Akkana Peck]

Anthonyd is taking over Output.  I'm not sure if this is still a problem or not
-- Naoki, can you comment?

Comment 9

[nhottanscp]

This is still reproducible with win32 branch build 2000-10-24-09-MN6.

Comment 10

[nhottanscp]

Attachment: test html I used as a signature

Comment 11

[Akkana Peck]

The reassign apparently didn't work -- trying again.

Comment 12

[bsharma]

updated qa contact.

Comment 13

[rubydoo123]

--> kin

Comment 14

[rubydoo123]

removing myself from the cc list

Comment 15

[nhottanscp]

Attachment: Changed nsPlainTextSerializer to try NCR if CER conversion fails.

Comment 16

[nhottanscp]

kin, could you review the patch?

Comment 17

[nhottanscp]

akkana, could you review the patch?

Comment 18

[Akkana Peck]

Comment on attachment 91430 [details] [diff] [review]
Changed nsPlainTextSerializer to try NCR if CER conversion fails.

Looks reasonable, r=akkana.  
Does str have the '#' in it, and if so, would it be safer to skip over that
character if ToInteger isn't guaranteed to?  But if it works this way, it can
only make things better.

Comment 19

[nhottanscp]

ToInteger() skips '#', I actually copied the code from the parser.
http://lxr.mozilla.org/seamonkey/source/htmlparser/src/nsHTMLTokens.cpp#2183

Comment 20

[kinmoz]

Comment on attachment 91430 [details] [diff] [review]
Changed nsPlainTextSerializer to try NCR if CER conversion fails.

So I'm a little curious, if an entity is not in our conversion table, say we
have &foobar;, we will try to convert that to an int, will ToInteger() return a
zero in that case? Is that ok?

Comment 21

[nhottanscp]

I think it's better to check if the first character is '#'.

Comment 22

[nhottanscp]

Attachment: Check if the fist character is '#'.

Comment 23

[nhottanscp]

akkana, could you review the new patch?

Comment 24

[Akkana Peck]

Comment on attachment 91838 [details] [diff] [review]
Check if the fist character is '#'.

I have a vague and possibly incorrect memory that it's better (faster/more
efficient) to use str.First() rather than str[0].  Can you check with jag or
scc on that?  Or maybe Kin knows.  Otherwise r=akkana.

Comment 25

[kinmoz]

Comment on attachment 91838 [details] [diff] [review]
Check if the fist character is '#'.

sr=[e-mail]

With the |str[0]| to |str.First()| change as akkana and scc suggest.

As a side note, I'm wondering if we should just write out the entity string in
the case where |entity == -1 && str.First() != '#'|? Not required for this bug,
but just a thought.

Comment 26

[nhottanscp]

Attachment: Change str[0] to str.First().

Comment 27

[nhottanscp]

Comment on attachment 91886 [details] [diff] [review]
Change str[0] to str.First().

copy r/sr

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 91886 [details] [diff] [review]
Change str[0] to str.First().

a=asa (on behalf of drivers) for checkin to 1.1

Comment 29

[nhottanscp]

checked in to the trunk