Bug 46251

Created attachment 73735 [details] diff

I'll take a look at this one. Feel free to assign it to me. I've noticed a couple of things. 1. fast/js/encode-URI-test.html is passing for me fast/js/encode-URI-test.html passed for me on Linux Qt. $ ./run-webkit-tests --debug LayoutTests/fast/js/encode-URI-test.html Running build-dumprendertree Running tests from /home/jwild/dev/webkit/WebKit.2011.03.17/LayoutTests Testing 1 test cases. fast/js . 1.05s total testing time all 1 test cases succeeded 2. Looks like "\uFFFF" in encoded as we would expect, but turns into a '?' mark as it is written the the actual.txt file. Need to track that down. QtTestBrowser HTML Dump of ":\uFFFF:" </body></html> Content-Type: text/plain layer at (0,0) size 800x600 RenderView at (0,0) size 800x600 layer at (0,0) size 800x600 RenderBlock {HTML} at (0,0) size 800x600 RenderBody {BODY} at (8,8) size 784x584 RenderBlock {P} at (0,0) size 784x21 RenderText {#text} at (0,0) size 34x21 text run at (0,0) width 34: "Start" RenderBlock {DIV} at (0,37) size 784x21 RenderInline {SPAN} at (0,0) size 14x21 RenderText {#text} at (0,0) size 14x21 text run at (0,0) width 14: ":\x{FFFF}:" RenderBR {BR} at (14,16) size 0x0 $ $B/DumpRenderTree LayoutTests/fast/js/switch-behaviour.html ... PASS characterSwitch('?') is "default"

This doesn't seem to be an error, but it is different behavior on Qt than on other platforms. Turns out that QString.toUtf8 converts reserved (non character) values like 0xFFFF to a replacement character of '?' (0x3f). Not sure if we want to change these tests or make separate test results for Qt. Seems better to me to have a definite char in the output, like '?' than to rely on how 0xFFFF will be displayed and/or diffed. /Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp void DumpRenderTree::dump() { ... QString markup = mainFrame->toHtml(); fprintf(stdout, "Source:\n\n%s\n", markup.toUtf8().constData()); So it looks like toUtf8 converts 0xFFFF to '?', which is as specified for toUtf8. QByteArray QString::toUtf8 () const e Returns a UTF-8 representation of the string as a QByteArray. UTF-8 is a Unicode codec and can represent all characters in a Unicode string like QString. However, in the Unicode range, there are certain codepoints that are not considered characters. The Unicode standard reserves the last two codepoints in each Unicode Plane (U+FFFE, U+FFFF, U+1FFFE, U+1FFFF, U+2FFFE, etc.), as well as 16 codepoints in the range U+FDD0..U+FDDF, inclusive, as non-characters. If any of those appear in the string, they may be discarded and will not appear in the UTF-8 representation, or they may be replaced by one or more replacement characters.

Created attachment 86350 [details] Rebenchmark Qt files that display a non-char Unicode value of 0xFFFF as a replacement char ('?') Talked to Ossy (Csaba Osztrogonac) and decided that converting 0xFFFF char to '?' was reasonable. So just updated the results for the Qt Platform. Note that I did not see fast/js/encode-URI-test.html failing, so did not do anything for that test.

Comment on attachment 86350 [details] Rebenchmark Qt files that display a non-char Unicode value of 0xFFFF as a replacement char ('?') You're right fast/js/encode-URI-test.html really passes now. Please update expected result for http/tests/websocket/tests/bad-sub-protocol-non-ascii.html too, and remove tests from the Skipped list too, and I will r+ the patch. ;)

Created attachment 86769 [details] Update skip list and add results for bad-sub-protocol-non-ascii.html Thanks for pointing out the missing bad-sub-protocol-non-ascii.html results. Had the wrong directory when trying to run that test. Note I did not update platform/qt-wk2/Skipped, since I was told it was not relevant.

Comment on attachment 86769 [details] Update skip list and add results for bad-sub-protocol-non-ascii.html View in context: https://bugs.webkit.org/attachment.cgi?id=86769&action=review LGTM, r=me with a little changelog modification. I'll do it, and land it manually. > LayoutTests/ChangeLog:17 > + be more reliable. Redid to update Skipped lists and add results > + for /bad-sub-protocol-non-ascii.html. > + > + Note: I did not update platform/qt-wk2/Skipped list. I was told > + that it did not matter. We don't need this comment in the ChangeLog: "Redid to update Skipped lists and add results for /bad-sub-protocol-non-ascii.html. Note: I did not update platform/qt-wk2/Skipped list. I was told that it did not matter."

Landed in http://trac.webkit.org/changeset/81866