96759 - [Composer] Charset can not be marked as specified

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Yuying Long]

Build: 08-22 trunk build on all platforms

Steps to reproduce:
1. Launch browser.

2. a. 
   1) Open Composer, View | Character Coding | More, select a charset other than
default charset(iso-8859-1 for English build...), e.g. x-jis, big5...
   2) Go View | Character Coding again, you will see the charset that you
specified in step a 1) is not marked.  I expected it will be marked.

   b.
   1) In browser, go some page that has different encoding than default, e.g. on
US build, go http://home.netscape.com/ja, then you will see x-jis will be marked
in Charset menu.
   2) File | Edit Page, to bring this page in Composer.
   3) Check the charset by going View | Character Coding, you will see the
charset will not be marked as anthing, and the s-jis is not been added into
charset menu.

Comment 1

[Roy Yokoyama]

assigning to jbetak

Comment 2

[[e-mail] (away - not reading bugmail)]

accepting

Comment 3

[Frank Tang]

mark as m0.9.7

Comment 4

[Frank Tang]

jbetak's contract is up. Bulk move bugs to ftang

Comment 5

[Frank Tang]

ui issue, rchen- please try.

Comment 6

[Yuying Long]

Nominating as nsbeta1 - it block many Composer charset feature.

Comment 7

[rchen]

It looks like "composer" is excluded in the eventlistener in charsetOverlay.js:

var wintype = document.firstChild.getAttribute('windowtype');
if (window && (wintype == "navigator:browser"))
{
    var contentArea = window.document.getElementById("appcontent");
    if (contentArea)
        contentArea.addEventListener("load", charsetLoadListener, true);
}
else
{
    var arrayOfStrings = wintype.split(":");
    if (window && arrayOfStrings[0] == "mail") 
    {
        var messageContent = window.document.getElementById("messagepane");
        if (messageContent)
            messageContent.addEventListener("load", mailCharsetLoadListener, 
true);
    }
}

Comment 8

[Frank Tang]

if you add a alert(wintype);
what will you got in the composer ?

Comment 9

[rchen]

I used dump to debug. It showed "composer:html" for composer. There could be 
some otherthing missing on the side of editorshell object. I am looking into it.

Comment 10

[Frank Tang]

try change
if (window && (wintype == "navigator:browser"))
to
if (window && ((wintype == "navigator:browser")||(wintype == "composer:html")))

Comment 11

[rchen]

I tried it already. It didn't work.  

Comment 12

[rchen]

Attachment: patch for adding composer event listener and force editor menu charset change in ComposerMultiplexHandler

Comment 13

[rchen]

ftang, can you please review it?

Comment 14

[rchen]

Naoki, can you help me test it on MAC? and review the patch? Thanks.

Comment 15

[nhottanscp]

I think gLastComposerCharset is not necessary. It is done that way for browser
which loads more than one page which is not the case for composer.
Could you try to remove gLastComposerCharset and see if it works?

Comment 16

[rchen]

gLastComposerCharset removed. It works fine. 

Comment 17

[rchen]

Attachment: new patch without gLastComposerCharset

Comment 18

[nhottanscp]

Comment on attachment 63897 [details] [diff] [review]
new patch without gLastComposerCharset

r=nhotta

Comment 19

[rchen]

alecf, can you sr?

Comment 20

[Alec Flett]

Comment on attachment 63897 [details] [diff] [review]
new patch without gLastComposerCharset

sr=alecf
we really need a better way to hook up the charset. 
For instance, what does chatzilla do? or the dom inspector? or the javascript
debugger? This needs to be dynamic so that each client can react to the charset
menu

Comment 21

[rchen]

Checkin. 

Comment 22

[Yuying Long]

On 01-15 trunk build/WinXP:
The problems in original report has been fixed.  However, the new charset is
only marked in UI but not in source code.

I'll mark this one as verified and open a new bug for the left over problem.

Comment 23

[Yuying Long]

Somehow, I don't see the left over problem any more...