Bug 46251 - [Qt] Some Unicode tests fail with Qt version >= 4.7.0
Summary: [Qt] Some Unicode tests fail with Qt version >= 4.7.0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged, Regression
Depends on:
Blocks: 46248
  Show dependency treegraph
 
Reported: 2010-09-22 02:03 PDT by Csaba Osztrogonác
Modified: 2011-05-09 14:56 PDT (History)
3 users (show)

See Also:


Attachments
diff (7.10 KB, patch)
2010-11-12 06:54 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Rebenchmark Qt files that display a non-char Unicode value of 0xFFFF as a replacement char ('?') (11.59 KB, patch)
2011-03-21 12:07 PDT, Joe Wild
ossy: review-
ossy: commit-queue-
Details | Formatted Diff | Diff
Update skip list and add results for bad-sub-protocol-non-ascii.html (14.65 KB, patch)
2011-03-24 07:04 PDT, Joe Wild
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Csaba Osztrogonác 2010-11-12 06:54:33 PST
Created attachment 73735 [details]
diff
Comment 2 Joe Wild 2011-03-20 11:36:29 PDT
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"
Comment 3 Joe Wild 2011-03-21 08:08:22 PDT
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.
Comment 4 Joe Wild 2011-03-21 12:07:06 PDT
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 5 Csaba Osztrogonác 2011-03-23 12:39:59 PDT
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. ;)
Comment 6 Joe Wild 2011-03-24 07:04:20 PDT
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 7 Csaba Osztrogonác 2011-03-24 07:14:06 PDT
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."
Comment 8 Csaba Osztrogonác 2011-03-24 07:26:58 PDT
Landed in http://trac.webkit.org/changeset/81866