161479 - [trunk]JavaScript string always converted to UTF-8 inside a Windows-1251 page

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Ivan Sagalaev]

In document written in Windows-1251 strings that appear as JavaScript string
constant are converted in UTF-8 unlike the rest of the page.

Comment 1

[Ivan Sagalaev]

Attachment: a testcase

1. Save the testcase on local disk (for <META HTTP-EQUIV> take effect)
2. Look into the source: both strings in HREF attribute and between <A></A>
should look identical (though unreadable for someone because they are in
Russian)
3. Open the testcase in browser and click on a link.
4. Compare two string: the one in alert window looks converted in UTF-8.

Comment 2

[Anthony DeRobertis]

WFM 2002072204 Linux. The alert looks identical to both the source and the
displayed text in the browser window (less font size differences, of course).

Comment 3

[Yuying Long]

It seem works for me either, I don't see the russian characters are displayed
much difference between html file and alert window.

Reporter: could you please attach a screen shot for the problem? thanks!

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: screenshot

I'm seeing this in Linux build 2002-08-05-08...  both locally and from
bugzilla.

Comment 5

[Ivan Sagalaev]

On my system (win98) it looks pretty much like on Boris' screenshot. Sorry, I
forgot to specify my build id: 2002080108

Comment 6

[Ivan Sagalaev]

Attachment: screenshot on Win98

Comment 7

[Yuying Long]

Hmm, I don't have any problem on my WinXP, also on WinME even though with WinME
the characters in Alert window looks like wider than they are in html file.

I tried it on linux RH7.2, it has similar result as in WinME, except miss the
last charcter"y" in alert window.

But I didn't see the garbage display like the 2 screen shot before with both case.

I'm going to confirm it now in order to get more investigation.

Comment 8

[ajbu]

On windows XP,
Build 1.0 2002053012 doesn't show this problem, testcase works fine.
On build 2002081409 running on the same computer, the testcase alert produces 
garbage

Comment 9

[Yuying Long]

I saw the garbage display in 08-15 trunk build but not branch build / WinME.

Comment 10

[ajbu]

can this be related to the renaming of nsISupportsWString / nsISupportsString 
bug 157624? I had other javascript problems with strings after this was 
checked in.

Comment 11

[Yuying Long]

Dup. bugs keep come in. Nominate as nsbeta1.

Comment 12

[Prisacari Sergey]

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

Comment 13

[Prisacari Sergey]

The problem is definitely with javascipt string constants in href attribute of
<a> tag. I mean the problem is simply with href attribute, but the only way to
encounter problems with it is using javascript inline functions.

Comment 14

[Eugene Savitsky]

Adding regression. And nominating for 1.2alfa.

Comment 15

[Ivan Sagalaev]

BTW, On Win98 2002090208 testcase now refuses to work :-(. It ignores
JavaScript: URL in <a href=...> and just reloads the same page instead.

Should I file a new bug or am I missing something?

Comment 16

[Ivan Sagalaev]

Oh... Another 'BTW'.
This bug looks very similar to bug 147991 which was filed earlier. May be we
should resolve this as a duplicate?

Comment 17

[Ivan Sagalaev]

Followup to comment #15:

The testcase malfunction can be traced in the status bar where 

javascript:window.alert('...');

looks like 

javascript:///window.alert('...');

But! This appears only if window.alert contains parameter spelled in cyrillic
letters. If I change this to, say, 

javascript:window.alert('Test');

then popup jumps up instantly upon clicking this thing. 

Comment 18

[Prisacari Sergey]

Because this bug has more comments and testcase it worth marking bug 147991 as dup

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It seems like there's weird stuff happening in
nsJSProtocolHandler::EnsureUTF8Spec.  I'm seeing (using \uNN for nonprintable
characters) it be called some of the time with the input:

Input spec (charset=windows-1251):
JavaScript:window.alert('\uD0\u9F\uD0\uBE\uD1\u87\uD0\uB5\uD0\uBC\uD1\u83 UTF-8?');

which leads it to the first early return.

However, sometimes it's called with the input:

Input spec (charset=windows-1251):
javascript:window.alert('%D0%9F%D0%BE%D1%87%D0%B5%D0%BC%D1%83 UTF-8?');

which leads it to run all the way through and produce the doubly-escaped output:

javascript:window.alert('%D0%A0%D1%9F%D0%A0%D1%95%D0%A1%E2%80%A1%D0%A0%C2%B5%D0%A0%D1%98%D0%A1%D1%93
UTF-8?');

which ends up being used.


From a low-level point of view, the percent-escaped input should have had
|aCharset| as UTF-8, not windows-1251.  That said, all this conversion strikes
me as awfully messy.

Comment 20

[nhottanscp]

Reassign to nhotta.

Comment 21

[nhottanscp]

> From a low-level point of view, the percent-escaped input should have had
> |aCharset| as UTF-8, not windows-1251.  That said, all this conversion strikes
> me as awfully messy.

This is actually about non escaped case. The current code assumes the URI as
UTF-8 if not percent escaped while many existing documents use raw 8bit as URI
without escaping. In order to support them, we assume UTF-8 only if the string
is UTF-8 otherwise use the document charset. About the conversion, there is a
utility function I can use so it can be simplified.

Comment 22

[nhottanscp]

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

Comment 23

[nhottanscp]

Attachment: Changed to use uconv util function to do the escape which check for valid UTF-8 and fallback to the doc charset.

Comment 24

[nhottanscp]

>This is actually about non escaped case.
This was actually wrong, the data is escaped as dbaron mentioned. The current
patch checks if the URI is UTF-8 or not either escaped or not escaped, so fixes
the problem.
I will take a look at the patch again tomorrow then ask for reviews.

Comment 25

[Asa Dotzler [:asa]]

drivers are interested in this for 1.2a adding to the 1.2 dependency list.

Comment 26

[nhottanscp]

Attachment: Added a support for 7bit encodings.

Comment 27

[Shanjian Li]

Add "HZ" to stateful charset set and them mark it as r=shanjian.

Comment 28

[nhottanscp]

Attachment: Added HZ as 7bit encoding

Comment 29

[nhottanscp]

Comment on attachment 98214 [details] [diff] [review]
Added HZ as 7bit encoding

r=shanjian

Comment 30

[Johnny Stenback (:jst)]

Comment on attachment 98214 [details] [diff] [review]
Added HZ as 7bit encoding

sr=jst

Comment 31

[Boris Zbarsky [:bzbarsky]]

This seems reasonable for now... once bug 166996 is fixed, we should revisit
this code, though....

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 98214 [details] [diff] [review]
Added HZ as 7bit encoding

a=dbaron for trunk checkin

Comment 33

[nhottanscp]

checked in to the trunk

Comment 34

[Yuying Long]

Verified fixed in 09-10 trunk build on windows and linux.

Comment 35

[Boris Zbarsky [:bzbarsky]]

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