|Summary:||Make fast\encoding\char-encoding.html not depend on the ability to have reference fragments on data URLs|
|Component:||Tools / Tests||Assignee:||Jungshik Shin <jshin>|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:|
Description xlyuan 2008-12-16 11:19:58 PST
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
Comment 2 Mark Rowe (bdash) 2008-12-17 17:40:54 PST
*** Bug 22883 has been marked as a duplicate of this bug. ***
Comment 3 Alexey Proskuryakov 2008-12-19 03:35:43 PST
The attachment is not a patch, please see <http://webkit.org/coding/contributing.html> for instructions.
Comment 4 Jungshik Shin 2008-12-29 12:55:03 PST
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
Comment 5 Alexey Proskuryakov 2008-12-29 13:16:32 PST
Could you please add a ChangeLog?
Comment 6 Jungshik Shin 2008-12-30 14:18:51 PST
Created attachment 26324 [details] patch with ChangeLog Oops. Now this one has the changelog.
Comment 7 Jungshik Shin 2008-12-30 14:23:05 PST
Created attachment 26325 [details] patch with changelog diff fixed
Comment 8 Alexey Proskuryakov 2008-12-31 00:00:09 PST
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
Comment 9 Jungshik Shin 2009-01-05 13:07:38 PST
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.
Comment 10 Alexey Proskuryakov 2009-01-11 01:41:27 PST
Committed revision 39787. I had to remove UTF-8 test results from expected results file to make the test pass.