RESOLVED FIXED 21581
http/tests/misc/frame-default-enc-same-domain.html should not depend on the presence of x-mac-cyrillic converter
https://bugs.webkit.org/show_bug.cgi?id=21581
Summary http/tests/misc/frame-default-enc-same-domain.html should not depend on the p...
Jungshik Shin
Reported 2008-10-13 16:14:25 PDT
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.
Attachments
patch (1.44 KB, patch)
2008-10-13 17:16 PDT, Jungshik Shin
ap: review-
patch with windows-1256 (1.59 KB, patch)
2008-10-14 14:30 PDT, Jungshik Shin
ap: review+
Jungshik Shin
Comment 1 2008-10-13 17:16:20 PDT
Created attachment 24333 [details] patch This patch replaces x-mac-cyrillic with windows-1251
Darin Adler
Comment 2 2008-10-13 18:01:55 PDT
Comment on attachment 24333 [details] patch r=me
Adam Barth
Comment 3 2008-10-14 01:19:22 PDT
Will land.
Adam Barth
Comment 4 2008-10-14 03:01:49 PDT
My Mac Mini died in the middle of testing this patch. I'm going to take it into the store tomorrow.
Alexey Proskuryakov
Comment 5 2008-10-14 04:06:03 PDT
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
Comment 6 2008-10-14 12:29:28 PDT
Reassigning to Jungshik based on comment above.
Jungshik Shin
Comment 7 2008-10-14 12:45:30 PDT
(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
Comment 8 2008-10-14 13:04:38 PDT
(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
Comment 9 2008-10-14 14:30:26 PDT
Created attachment 24346 [details] patch with windows-1256 I replaced windows-1251 with windows-1256 (Arabic)
Jungshik Shin
Comment 10 2008-10-14 17:14:58 PDT
(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 11 2008-10-15 00:05:09 PDT
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
Comment 12 2008-10-21 13:46:16 PDT
Adam or Alexey, can you land the latest patch?
Adam Barth
Comment 13 2008-10-21 15:49:30 PDT
Yep. Assigned.
Adam Barth
Comment 14 2008-10-22 01:10:54 PDT
Note You need to log in before you can comment on or make changes to this bug.