88428 2012-06-06 10:00:41 -0700 REGRESSION (r115654): Opening many non-English WebArchives shows obvious encoding issues 2012-06-07 09:38:32 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Mac All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson abarth ap darin japhet rniwa oldest_to_newest 642562 0 beidson 2012-06-06 10:00:41 -0700 REGRESSION (r115654): Opening many non-English WebArchives shows obvious encoding issues http://trac.webkit.org/changeset/115654 is the change that caused this. The claim in the patch was that this was just a refactor with no change in behavior, which is false. If we don't figure this one out pretty soon, I'm going to try my best to roll it out. In Radar as <rdar://problem/11575112> 642564 1 beidson 2012-06-06 10:02:05 -0700 To reproduce: 1 - Go to http://www.dni.ru/ 2 - Save as a WebArchive. 3 - In a WebKit nightly post-115654, open that WebArchive The encoding issues will be obvious. 642692 2 japhet 2012-06-06 12:28:11 -0700 (In reply to comment #1) > To reproduce: > > 1 - Go to http://www.dni.ru/ > 2 - Save as a WebArchive. > 3 - In a WebKit nightly post-115654, open that WebArchive > > The encoding issues will be obvious. Yep. Looks like in r115654, I dropped the setEncoding() call for archive loads based on the the main archive resource's text encoding. Should be a 1-line fix, working on test now. 642722 3 japhet 2012-06-06 13:04:36 -0700 (In reply to comment #2) > (In reply to comment #1) > > To reproduce: > > > > 1 - Go to http://www.dni.ru/ > > 2 - Save as a WebArchive. > > 3 - In a WebKit nightly post-115654, open that WebArchive > > > > The encoding issues will be obvious. > > Yep. Looks like in r115654, I dropped the setEncoding() call for archive loads based on the the main archive resource's text encoding. Should be a 1-line fix, working on test now. Is there a way to load a .webarchive as a layout test? I can't seem to find an example, and the problem is with loading, not creating the webarchive. 642743 4 beidson 2012-06-06 13:23:28 -0700 (In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > To reproduce: > > > > > > 1 - Go to http://www.dni.ru/ > > > 2 - Save as a WebArchive. > > > 3 - In a WebKit nightly post-115654, open that WebArchive > > > > > > The encoding issues will be obvious. > > > > Yep. Looks like in r115654, I dropped the setEncoding() call for archive loads based on the the main archive resource's text encoding. Should be a 1-line fix, working on test now. > > Is there a way to load a .webarchive as a layout test? I can't seem to find an example, and the problem is with loading, not creating the webarchive. Great question. I'll poke around. 642769 5 beidson 2012-06-06 13:47:09 -0700 (In reply to comment #3) > Is there a way to load a .webarchive as a layout test? I can't seem to find an example, and the problem is with loading, not creating the webarchive. I can't find anything. On the surface, adding such support would involve mucking around in the mess of perl and python that make up the run-webkit-tests infrastructure, in order to recognize webarchives as tests to be run. But one shortcut idea would be to add a test which loads a webarchive in an iframe. I haven't tested this yet, but it seems like it should work. 642942 6 146141 beidson 2012-06-06 16:45:53 -0700 Created attachment 146141 Patch v1 - Fix + test 642982 7 beidson 2012-06-06 17:12:18 -0700 http://trac.webkit.org/changeset/119643 642983 8 beidson 2012-06-06 17:12:28 -0700 (In reply to comment #7) > http://trac.webkit.org/changeset/119643 I meant http://trac.webkit.org/changeset/119644 643081 9 rniwa 2012-06-06 19:10:11 -0700 This test has been failing on Chromium. http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Floader%2Fwebarchive-encoding-respected.html (Sorry, there was a some TestExpectations error and the test was skipped momentarily) 643107 10 beidson 2012-06-06 19:42:32 -0700 ACK - My apologies, it should basically be skipped on all non-Apple ports. I'm trying to get a checkout up and running where I am right now, but if anyone can beat me to it please feel free. 643114 11 rniwa 2012-06-06 19:48:06 -0700 (In reply to comment #10) > ACK - My apologies, it should basically be skipped on all non-Apple ports. > > I'm trying to get a checkout up and running where I am right now, but if anyone can beat me to it please feel free. Ah, okay. Could you add them to relevant Skipped and TestExpectation files? See http://trac.webkit.org/changeset/119665 643115 12 beidson 2012-06-06 19:50:02 -0700 (In reply to comment #10) > ACK - My apologies, it should basically be skipped on all non-Apple ports. Much easier to just install the test in the platform/mac directory for now, which I'll do soon. 643117 13 beidson 2012-06-06 19:52:33 -0700 (In reply to comment #11) > (In reply to comment #10) > > ACK - My apologies, it should basically be skipped on all non-Apple ports. > > > > I'm trying to get a checkout up and running where I am right now, but if anyone can beat me to it please feel free. > > Ah, okay. Could you add them to relevant Skipped and TestExpectation files? > See http://trac.webkit.org/changeset/119665 Since they're skipped for now and the bots are not longer imminently red, I'll work out of 88481 in the morning to move the test and cleanup the expectations that were checked in. 643118 14 rniwa 2012-06-06 19:53:14 -0700 (In reply to comment #12) > (In reply to comment #10) > > ACK - My apologies, it should basically be skipped on all non-Apple ports. > > Much easier to just install the test in the platform/mac directory for now, which I'll do soon. Okay, could you revert my patch & close the bug 88481 when you do that? By the way, I think chromium port runs tests in platform/mac :( 643623 15 beidson 2012-06-07 09:36:18 -0700 (In reply to comment #14) > (In reply to comment #12) > > (In reply to comment #10) > > > ACK - My apologies, it should basically be skipped on all non-Apple ports. > > > > Much easier to just install the test in the platform/mac directory for now, which I'll do soon. > > Okay, could you revert my patch & close the bug 88481 when you do that? By the way, I think chromium port runs tests in platform/mac :( That's bizarre considering they don't use the "platform/mac" WebKit... I'll leave Chromium/mac skipped. 643627 16 beidson 2012-06-07 09:38:32 -0700 (In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #12) > > > (In reply to comment #10) > > > > ACK - My apologies, it should basically be skipped on all non-Apple ports. > > > > > > Much easier to just install the test in the platform/mac directory for now, which I'll do soon. > > > > Okay, could you revert my patch & close the bug 88481 when you do that? By the way, I think chromium port runs tests in platform/mac :( > > That's bizarre considering they don't use the "platform/mac" WebKit... I'll leave Chromium/mac skipped. Nah, we're good: // Run the Mac-specific platform tests, but only to check for crashes. WONTFIX : platform/mac = FAIL PASS TIMEOUT 146141 2012-06-06 16:45:53 -0700 2012-06-06 16:47:28 -0700 Patch v1 - Fix + test patch.txt text/plain 3644 beidson ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA4MDEzOTFmLi42ZjM1NjQ1IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTItMDYt MDYgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAgICAgIDxyZGFyOi8v cHJvYmxlbS8xMTU3NTExMj4gYW5kIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD04ODQyOAorICAgICAgICBSRUdSRVNTSU9OIChyMTE1NjU0KTogT3BlbmluZyBtYW55IG5v bi1FbmdsaXNoIFdlYkFyY2hpdmVzIHNob3dzIG9idmlvdXMgZW5jb2RpbmcgaXNzdWVzCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBmYXN0L2xvYWRl ci9yZXNvdXJjZXMvdXRmOC1lbmNvZGVkLndlYmFyY2hpdmU6IEFkZGVkLgorICAgICAgICAqIGZh c3QvbG9hZGVyL3dlYmFyY2hpdmUtZW5jb2RpbmctcmVzcGVjdGVkLWV4cGVjdGVkLnR4dDogQWRk ZWQuCisgICAgICAgICogZmFzdC9sb2FkZXIvd2ViYXJjaGl2ZS1lbmNvZGluZy1yZXNwZWN0ZWQu aHRtbDogQWRkZWQuCisKIDIwMTItMDYtMDYgIElseWEgVGlraG9ub3Zza3kgIDxsb2lzbG9AY2hy b21pdW0ub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgcmViYXNlbGluZSBmb3IgcjExOTU5MS4K ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvbG9hZGVyL3Jlc291cmNlcy91dGY4LWVuY29k ZWQud2ViYXJjaGl2ZSBiL0xheW91dFRlc3RzL2Zhc3QvbG9hZGVyL3Jlc291cmNlcy91dGY4LWVu Y29kZWQud2ViYXJjaGl2ZQpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi5jY2Rh NzY4CkJpbmFyeSBmaWxlcyAvZGV2L251bGwgYW5kIGIvTGF5b3V0VGVzdHMvZmFzdC9sb2FkZXIv cmVzb3VyY2VzL3V0ZjgtZW5jb2RlZC53ZWJhcmNoaXZlIGRpZmZlcgpkaWZmIC0tZ2l0IGEvTGF5 b3V0VGVzdHMvZmFzdC9sb2FkZXIvd2ViYXJjaGl2ZS1lbmNvZGluZy1yZXNwZWN0ZWQtZXhwZWN0 ZWQudHh0IGIvTGF5b3V0VGVzdHMvZmFzdC9sb2FkZXIvd2ViYXJjaGl2ZS1lbmNvZGluZy1yZXNw ZWN0ZWQtZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAuLmEw ZWM3OGQKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2xvYWRlci93ZWJhcmNo aXZlLWVuY29kaW5nLXJlc3BlY3RlZC1leHBlY3RlZC50eHQKQEAgLTAsMCArMSw3IEBACitUaGUg d2ViYXJjaGl2ZSBpbiB0aGlzIGlmcmFtZSBpcyB1dGYtOCBlbmNvZGVkIGFuZCB3aWxsIG9ubHkg ZGlzcGxheSBwcm9wZXJseSBpZiB0aGUgd2ViYXJjaGl2ZSdzIGVuY29kaW5nIGlzIHJlc3BlY3Rl ZC4KKworCistLS0tLS0tLQorRnJhbWU6ICc8IS0tZnJhbWVQYXRoIC8vPCEtLWZyYW1lMC0tPi0t PicKKy0tLS0tLS0tCitUaGlzIGNvbnRlbnQgaXMgVVRGLTggZW5jb2RlZCBhbmQgaW5jbHVkZXMg YSBIZWJyZXcgUmVzaCBjaGFyYWN0ZXIgLT4g16gKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zh c3QvbG9hZGVyL3dlYmFyY2hpdmUtZW5jb2RpbmctcmVzcGVjdGVkLmh0bWwgYi9MYXlvdXRUZXN0 cy9mYXN0L2xvYWRlci93ZWJhcmNoaXZlLWVuY29kaW5nLXJlc3BlY3RlZC5odG1sCm5ldyBmaWxl IG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAuLmRjNDBjM2YKLS0tIC9kZXYvbnVsbAorKysgYi9M YXlvdXRUZXN0cy9mYXN0L2xvYWRlci93ZWJhcmNoaXZlLWVuY29kaW5nLXJlc3BlY3RlZC5odG1s CkBAIC0wLDAgKzEsOCBAQAorPHNjcmlwdD4KK2lmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xs ZXIpIHsKKwlsYXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7CisJbGF5b3V0VGVzdENv bnRyb2xsZXIuZHVtcENoaWxkRnJhbWVzQXNUZXh0KCk7CQorfQorPC9zY3JpcHQ+CitUaGUgd2Vi YXJjaGl2ZSBpbiB0aGlzIGlmcmFtZSBpcyB1dGYtOCBlbmNvZGVkIGFuZCB3aWxsIG9ubHkgZGlz cGxheSBwcm9wZXJseSBpZiB0aGUgd2ViYXJjaGl2ZSdzIGVuY29kaW5nIGlzIHJlc3BlY3RlZC48 YnI+Cis8aWZyYW1lIHNyYz0icmVzb3VyY2VzL3V0ZjgtZW5jb2RlZC53ZWJhcmNoaXZlIj48L2lm cmFtZT4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJD b3JlL0NoYW5nZUxvZwppbmRleCAwMTA2MGE4Li4wY2JlZmFhIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMg KzEsMTUgQEAKKzIwMTItMDYtMDYgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgor CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xMTU3NTExMj4gYW5kIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD04ODQyOAorICAgICAgICBSRUdSRVNTSU9OIChyMTE1NjU0 KTogT3BlbmluZyBtYW55IG5vbi1FbmdsaXNoIFdlYkFyY2hpdmVzIHNob3dzIG9idmlvdXMgZW5j b2RpbmcgaXNzdWVzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgVGVzdDogZmFzdC9sb2FkZXIvd2ViYXJjaGl2ZS1lbmNvZGluZy1yZXNwZWN0ZWQuaHRt bAorCisgICAgICAgICogbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNv cmU6OkRvY3VtZW50TG9hZGVyOjpjb21taXREYXRhKTogUHJvcGVybHkgc2V0IHRoZSBtYWluIHJl c291cmNlIGVuY29kaW5nIGZyb20gdGhlIHdlYmFyY2hpdmUuCisKIDIwMTItMDYtMDYgIEp1bGll biBDaGFmZnJhaXggIDxqY2hhZmZyYWl4QHdlYmtpdC5vcmc+CiAKICAgICAgICAgQWRkIHN1cHBv cnQgZm9yIGRpcmVjdGlvbiBvbiB0YWJsZSByb3cgZ3JvdXAgd2l0aCBjb2xsYXBzaW5nIGJvcmRl cnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9Eb2N1bWVudExvYWRlci5jcHAg Yi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIuY3BwCmluZGV4IGJhNzliOTAu LjMwMmE3YWQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9Eb2N1bWVudExvYWRl ci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcApAQCAt MzQyLDYgKzM0MiwxMCBAQCB2b2lkIERvY3VtZW50TG9hZGVyOjpjb21taXREYXRhKGNvbnN0IGNo YXIqIGJ5dGVzLCBzaXplX3QgbGVuZ3RoKQogICAgICAgICBpZiAoZW5jb2RpbmcuaXNOdWxsKCkp IHsKICAgICAgICAgICAgIHVzZXJDaG9zZW4gPSBmYWxzZTsKICAgICAgICAgICAgIGVuY29kaW5n ID0gcmVzcG9uc2UoKS50ZXh0RW5jb2RpbmdOYW1lKCk7CisjaWYgRU5BQkxFKFdFQl9BUkNISVZF KQorICAgICAgICAgICAgaWYgKG1fYXJjaGl2ZSAmJiBtX2FyY2hpdmUtPnR5cGUoKSA9PSBBcmNo aXZlOjpXZWJBcmNoaXZlKQorICAgICAgICAgICAgICAgIGVuY29kaW5nID0gbV9hcmNoaXZlLT5t YWluUmVzb3VyY2UoKS0+dGV4dEVuY29kaW5nKCk7CisjZW5kaWYKICAgICAgICAgfQogICAgICAg ICBtX3dyaXRlci5zZXRFbmNvZGluZyhlbmNvZGluZywgdXNlckNob3Nlbik7CiAgICAgfQo=