286365 - After manually resetting encoding, "View Source" uses original setting

Status: RESOLVED FIXED

(Toolkit :: View Source, defect)

Description

[Ning Cao]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

Firefox has options to let user choose the character encoding for current page.

When both the menu items of "[view]->[Character Encoding]" and
"[view]->[Character Encoding]->[Auto-Detected]" are set to "GB2312", the browser
display a GB2312 page in GB2312. It works fine.

But when you click "Page Source" in the "view" menu or "View Page Source" in the
popup menu, the "view-source" window displays the source in ISO-8859-1.

It will be better to display the page source use the same encoding as the web
page being viewed by default.

Seems IE 6.0.2900 do this better.

Reproducible: Always

Steps to Reproduce:
1. Open a webpage whose encoding is not western characters.
2. View its source


Actual Results:  
The page source is displayed in ISO-8859-1.

Expected Results:  
The same character encoding as the web page being viewed.

Comment 1

[Simon Montagu :smontagu]

At http://cn.yahoo.com/ View source shows GB2312 just like browser. At what site
are you seeing the bug?

Comment 2

[Ning Cao]

(In reply to comment #1)
> At http://cn.yahoo.com/ View source shows GB2312 just like browser. At what site
> are you seeing the bug?

Yes, http://cn.yahoo.com/ works fine.
please try  http://www.csr.uky.edu/~ningcao/286365.html

I am not sure if firefox "view source" decides the encoding based on headers of
that page instead what encoding I choose for that page. It is ok to choose
encoding in the same manner with the browser, but (I think) it will be better to
use the same encoding as what the user choose in browser.

I am not sure if firefox(browser) failed to decide the encoding of that page.
It always switch to iso-8859-1 instead of gb2312 what that page is loaded.
IE, mozilla(browser) also failed to switch to gb2312 when that page is loaded.
There are content="text/html; charset=gb2312" in the header tag of the file.
It maybe because that the webserver on that machine send a header of "ISO-8859-1".

Comment 3

[Simon Montagu :smontagu]

(In reply to comment #2)

> It maybe because that the webserver on that machine send a header of "ISO-8859-1".

Yes, when the charset in the <meta> conflicts with that in the HTTP headers, we
follow the HTTP headers, which is what standards say we should do. So the bug
here is that after you reset the charset manually, View Source follows the
original charset and doesn't recognise the change.

Comment 4

[Simon Montagu :smontagu]

The override charset gets set correctly at
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/viewsource.js#125

Something must be resetting it later.

Comment 5

[Jason Barnabe (np)]

Attachment: patch

This works, though I'm not 100% sure what the difference is. I also defined
BrowserSetDefaultCharacterSet because charsetOverlay.js was complaining about
it not being there when you change the charset.

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 180853 [details] [diff] [review]
patch

in browser.js this is a stub, I'd rather just kill the stub, kill the stub that calls it even though it does nothing, and fix the callers of that stub

Comment 7

[Jason Barnabe (np)]

Attachment: patch with stubicide

Comment 8

[Asaf Romano (gone)]

Comment on attachment 207574 [details] [diff] [review]
patch with stubicide

>Index: toolkit/components/viewsource/content/viewSource.js
>===================================================================
>     if (window.arguments.length >= 2) {
>       arg = window.arguments[1];
> 
>       try {
>         if (typeof(arg) == "string" && arg.indexOf('charset=') != -1) {
>           var arrayArgComponents = arg.split('=');
>           if (arrayArgComponents) {
>             //we should "inherit" the charset menu setting in a new window
>-            getMarkupDocumentViewer().defaultCharacterSet = arrayArgComponents[1];
>+            var docCharset = getBrowser().docShell.QueryInterface(
>+                             Components.interfaces.nsIDocCharset);

nit: make this.
>+            var docCharset = getBrowser().docShell.QueryInterface
>+                               (Components.interfaces.nsIDocCharset);


Looks ok, but i would like Simon's approval on this too.

Comment 9

[Jason Barnabe (np)]

Attachment: fix nit

Comment 10

[Asaf Romano (gone)]

mozilla/browser/base/content/browser.js 1.479.2.206
mozilla/toolkit/components/viewsource/content/viewSource.js 1.13.4.4
mozilla/toolkit/content/charsetOverlay.js 1.10.2.1

Comment 11

[Asaf Romano (gone)]

oops, backed out from the branch...

trunk checkin:
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.707; previous revision: 1.706
done
Checking in toolkit/components/viewsource/content/viewSource.js;
/cvsroot/mozilla/toolkit/components/viewsource/content/viewSource.js,v  <--  viewSource.js
new revision: 1.17; previous revision: 1.16
done
Checking in toolkit/content/charsetOverlay.js;
/cvsroot/mozilla/toolkit/content/charsetOverlay.js,v  <--  charsetOverlay.js
new revision: 1.12; previous revision: 1.11
done

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This fix brakes the View Source on Feed View that is writing non-UTF-8 RSS.
See http://forum.mozilla.gr.jp/news.rdf

And this also breaks the View Source on JS created document.
See http://www2.wbs.ne.jp/%7echado/work/js_write_sjis/js_write_sjis.html

Cannot check the charset is changed by user?

Comment 13

[Tae Kim]

The problem is that the rdf is displayed in UTF-8 no matter what the original encoding is. It looks like you can't even forcibly change it.

We need to either 1) Display the feed as the original encoding or 2) roll this change back out so that view source tries to pick the most appropriate encoding instead of inheriting (the stubs can stay deleted though).

See my comment on bug 358673 regarding the JS created document.

Comment 14

[Boris Zbarsky [:bzbarsky]]

It looks like this most probably caused bug 382074.

Seamonkey folks: once that's fixed you might want something like this, maybe.