Bug 21581 – http/tests/misc/frame-default-enc-same-domain.html should not depend on the presence of x-mac-cyrillic converter

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jungshik Shin]

http/tests/misc/frame-default-enc-same-domain.html has meta charset declaration for x-mac-cyrillic. However, the test is for the inheritance of charset from the parent frame.  Some projects using webkit (e.g. Chrome) use their own custom build of ICU without x-mac-cyrillic converter.  It's not a good idea to make a single test depend on something other than what it intends to test.

[Jungshik Shin]

Created attachment 24333 [details]
patch 

This patch replaces x-mac-cyrillic with windows-1251

[Darin Adler]

Comment on attachment 24333 [details]
patch 

r=me

[Adam Barth]

Will land.

[Adam Barth]

My Mac Mini died in the middle of testing this patch.  I'm going to take it into the store tomorrow.

[Alexey Proskuryakov]

Comment on attachment 24333 [details]
patch 

Sorry, I specifically chose MacCyrillic to make this test work reasonably on my machine (where I have windows-1251 as a default encoding). I'd ask you to change this to some encoding that is unlikely to be user default for any WebKit developer.

On another note, I'm surprised that you can get away without supporting MacCyrillic - this encoding is in wide use on the Web. Many Russian sites re-encode their text to MacCyrillic on the fly if they see "Mac" in UA string (a practice that makes no sense in this age, but anyway). Maybe you don't get bug reports about this because you only have a Windows beta now?

[Adam Barth]

Reassigning to Jungshik based on comment above.

[Jungshik Shin]

(In reply to comment #5)
> (From update of attachment 24333 [details] [edit])
> Sorry, I specifically chose MacCyrillic to make this test work reasonably on my
> machine (where I have windows-1251 as a default encoding). I'd ask you to
> change this to some encoding that is unlikely to be user default for any WebKit
> developer.

Sure, I can change (to what encoding? I'll try to get the most unlikely one), but does it really matter in layout test? 

> On another note, I'm surprised that you can get away without supporting
> MacCyrillic - this encoding is in wide use on the Web. Many Russian sites
> re-encode their text to MacCyrillic on the fly if they see "Mac" in UA string
> (a practice that makes no sense in this age, but anyway). Maybe you don't get
> bug reports about this because you only have a Windows beta now?
 
I'm aware of the practice. I have little idea how widespread the practice is. mail.ru was one of them, but it does not do that any more, does it? Anyway, if the practice is still widespread, I'll add it back (before we release Mac version)  paying the price for one more item in the encoding list and a few more kBs in the ICU data file :-)

[Alexey Proskuryakov]

(In reply to comment #7)
> Sure, I can change (to what encoding? I'll try to get the most unlikely one),
> but does it really matter in layout test? 

Yes, we try to make layout tests work in the browser if possible. This reduces confusion and simplifies debugging in case the test breaks.

ISO-8859-5 is another example of a Cyrillic encoding that is unlikely to be default for many people.

> I'm aware of the practice. I have little idea how widespread the practice is.
> mail.ru was one of them, but it does not do that any more, does it? Anyway, if
> the practice is still widespread, I'll add it back (before we release Mac
> version)  paying the price for one more item in the encoding list and a few
> more kBs in the ICU data file :-)

I don't have any current examples. Besides mail.ru, anekdot.ru used to do the same until very recently, and these are some of the largest resources out there. Also, this was the default configuration of Russian Apache distribution, so I expect that sites doing this will be around for a long time.

[Jungshik Shin]

Created attachment 24346 [details]
patch with windows-1256

I replaced windows-1251 with windows-1256 (Arabic)

[Jungshik Shin]

(In reply to comment #9)
> Created an attachment (id=24346) [edit]
> patch with windows-1256
> 
> I replaced windows-1251 with windows-1256 (Arabic)

My patch and Alexey's reply 'crossed in mail'. Anyway, I guess windows-1256 should be as good as ISO-8859-5.  ISO-8859-15 would have been better :-)

As for Russian Apache (mod_charset), does the current version still do that? If so, at least we should ask them to not to do it any more. 

[Alexey Proskuryakov]

Comment on attachment 24346 [details]
patch with windows-1256

r=me

> As for Russian Apache (mod_charset), does the current version still do that? If
> so, at least we should ask them to not to do it any more. 

I used to think that it was no longer maintained, but it turns out to be not quite so. Anyway, according to documentation, the default for CharsetAgent is empty now.

[Jungshik Shin]

Adam or Alexey, can you land the latest patch? 

[Adam Barth]

Yep.  Assigned.

[Adam Barth]

Fixed in http://trac.webkit.org/changeset/37775