We must use TextEncoding(charset) to encode data URL again. Otherwise TextEncoding::encode() returns an empty string because encoding name is not specified. Also use "US-ASCII" as fallback charset as extractCharsetFromMediaType can be empty.
Created attachment 70909 [details] Patch
Comment on attachment 70909 [details] Patch Is there a regression test that covers this? If so, where is the patch to the expected results to show the progression?
(In reply to comment #2) > (From update of attachment 70909 [details]) > Is there a regression test that covers this? If so, where is the patch to the expected results to show the progression? We need this patch to pass Acid 3 test 97 (data URL).
(In reply to comment #2) > (From update of attachment 70909 [details]) > Is there a regression test that covers this? If so, where is the patch to the expected results to show the progression? r69183 passed all LayoutTests. I think we need to add a new test case for this.
Adding Patrick to CC.
Created attachment 74968 [details] Patch I added an additonal change to the patch (we don't need to declare the default charset twice). (In reply to comment #2) > (From update of attachment 70909 [details]) > Is there a regression test that covers this? If so, where is the patch to the expected results to show the progression? The reason for this is, that none of the current ports with a buildbot uses this functionality. I've run the GTK LayoutTests on the patch which introduced DataURL.cpp. I thought GTK uses the curl backend, but this was wrong and so I didn't see the regression :-(. I did a small test and changed a port with LayoutTests to use DataURL.cpp: This patch fixed over 130 layout tests. So I don't think that we need additional tests. We only need to run the existing ones.
@darin: ping
Comment on attachment 74968 [details] Patch The ChangeLog needs to mention that this fixes 130 layout tests, and for which port. Otherwise the change looks fine. (Every change requires a test, or explanation of why testing is impossible.) In this case, you have many tests, you just didn't explain that you're fixing them (which is importnat both for reviewers and folks looking back through teh svn history).
Created attachment 77507 [details] Patch
Comment on attachment 77507 [details] Patch What do other ports do when extractCharsetFromMediaType returns empty? Do they already assume ASCII? If so, how? Seems like it should be possible to write a test for this which would fail on other ports as well, no?
Comment on attachment 77507 [details] Patch 130! Wow!
> Seems like it should be possible to write a test for this which would fail on other ports as well, no? Not very many folks use this file.
Comment on attachment 77507 [details] Patch Clearing flags on attachment: 77507 Committed r84976: <http://trac.webkit.org/changeset/84976>
All reviewed patches have been landed. Closing bug.