Bug 37492 - Add more Mailto: URL regression tests
Summary: Add more Mailto: URL regression tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jonathan Dixon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-12 23:14 PDT by Jonathan Dixon
Modified: 2010-04-15 01:15 PDT (History)
4 users (show)

See Also:


Attachments
first patch (no expects file yet, need to integrate with new utilities.js methods) (2.61 KB, patch)
2010-04-12 23:44 PDT, Jonathan Dixon
abarth: review-
Details | Formatted Diff | Diff
patch 2 (refactored, still needs expects file) (2.37 KB, patch)
2010-04-13 15:25 PDT, Jonathan Dixon
no flags Details | Formatted Diff | Diff
Patch 3 - good to go? (3.59 KB, patch)
2010-04-13 23:49 PDT, Jonathan Dixon
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Dixon 2010-04-12 23:14:49 PDT
Add Mailto: support to URL regression tests
Comment 1 Jonathan Dixon 2010-04-12 23:44:08 PDT
Created attachment 53226 [details]
first patch (no expects file yet, need to integrate with new utilities.js methods)

just uploading patch to get place-holder for these tests into the system
Comment 2 WebKit Review Bot 2010-04-12 23:46:39 PDT
Attachment 53226 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Adam Barth 2010-04-13 01:05:15 PDT
Comment on attachment 53226 [details]
first patch (no expects file yet, need to integrate with new utilities.js methods)

This looks like a good start.  You might want to look at the query test to see how to write the test with an array (which is slightly easier to read and modify later).
Comment 4 Jonathan Dixon 2010-04-13 15:25:39 PDT
Created attachment 53287 [details]
patch 2 (refactored, still needs expects file)
Comment 5 Alexey Proskuryakov 2010-04-13 15:49:08 PDT
I think this bug has a wrong title. The patch doesn't add support for mailto: tests, it adds tests!

We don't need any special support for mailto, as far as I can tell, and we even have some tests already, such as fast/encoding/mailto-always-utf-8.html.
Comment 6 Jonathan Dixon 2010-04-13 23:49:03 PDT
Created attachment 53321 [details]
Patch 3 - good to go?
Comment 7 Adam Barth 2010-04-14 09:49:16 PDT
Comment on attachment 53321 [details]
Patch 3 - good to go?

Looks good.  One detail:

+// This test cannot be represented in JavaScript, as \0 is stripped before execution
+//  ["addr1\0addr2?foo", "addr1%00addr2?foo"],

You can actually do this test.  The trick to to escape the \ because these strings get evaled twice:

+//  ["addr1\\\0addr2?foo", "addr1%00addr2?foo"],
Comment 8 Adam Barth 2010-04-15 01:15:02 PDT
Committed r57636: <http://trac.webkit.org/changeset/57636>
Comment 9 Adam Barth 2010-04-15 01:15:19 PDT
I was wrong.  That doesn't seem to work either.  I've landed your patch as-is.  Thanks!