Bug 22882 – Make fast\encoding\char-encoding.html not depend on the ability to have reference fragments on data URLs

• Component: Tools / Tests
• Status: RESOLVED FIXED

1. Currently, fast\encoding\char-encoding.html depend on the ability to have reference fragments on data URLs, it makes the layout test on Chrome fails, modified char-encoding.html and added resources/dummy.html to make this test case can be passed on Chrome
2. Move Mac* encodings to char-encoding-mac.html

Created attachment 26058 [details]
Patch file

[Mark Rowe (bdash)]

*** Bug 22883 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

The attachment is not a patch, please see <http://webkit.org/coding/contributing.html> for instructions.

[Jungshik Shin]

Created attachment 26295 [details]
patch built on top of xlyuan's change

As in attachment 26058 [details] by Xiaolu, I used a local dummy file access instead of data: . While splitting char-encoding to two tests (char-encoding and char-encoding-mac), I moved the common functions to resources/char-encoding-utils.js

[Alexey Proskuryakov]

Could you please add a ChangeLog?

[Jungshik Shin]

Created attachment 26324 [details]
patch with ChangeLog

Oops. Now this one has the changelog.

[Jungshik Shin]

Created attachment 26325 [details]
patch with changelog diff fixed

[Alexey Proskuryakov]

Comment on attachment 26325 [details]
patch with changelog diff fixed

> +        possible to to skip it.

Typo: "to to".

> +        - Enable UTF-8 test for U+221A

How did you ensure that this is safe now? Did you test on Tiger?

This should really be mentioned in ChangeLog.

> +        * fast/encoding/resources/char-encoding-utils.js: Added.
> +        (encode):
> +        (testsDone):
> +        (processResult):
> +        (subframeLoaded):
> +        (loadTimedOut):
> +        (runTest):
> +        (testEncode):

We normally try to describe changes for each file, not just as a global overview. This is not very important in this case, but I think that you could omit function names for brevity, as you don't have descriptions of individual changes anyway.

r=me

[Jungshik Shin]

Created attachment 26437 [details]
updated patch with ap's review comments addressed

Moving r+ flag from the previous patch. I addressed ap's review comments:

I got rid of function names in ChangeLog. 
I also left alone the UTF-8 test case (i.e. it's still commented out) because I don't have an access to Tiger. I mistakenly thought that the issue was across platforms and not using 'data' url would resolve it.

[Alexey Proskuryakov]

Committed revision 39787.

I had to remove UTF-8 test results from expected results file to make the test pass.