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>
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.
(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.
(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.
(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.
(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.
Created attachment 146141 [details] Patch v1 - Fix + test
http://trac.webkit.org/changeset/119643
(In reply to comment #7) > http://trac.webkit.org/changeset/119643 I meant http://trac.webkit.org/changeset/119644
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)
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.
(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
(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.
(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.
(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 :(
(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.
(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