Bug 37492

Summary:

Add more Mailto: URL regression tests

Product:

WebKit

Reporter:

Jonathan Dixon <joth>

Component:

New Bugs

Assignee:

Jonathan Dixon <joth>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ap, dbates, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
first patch (no expects file yet, need to integrate with new utilities.js methods)	abarth: review-
patch 2 (refactored, still needs expects file)	none
Patch 3 - good to go?	abarth: review-

Jonathan Dixon

Reported 2010-04-12 23:14:49 PDT

Add Mailto: support to URL regression tests

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Jonathan Dixon

Comment 1 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

WebKit Review Bot

Comment 2 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.

Adam Barth

Comment 3 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).

Jonathan Dixon

Comment 4 2010-04-13 15:25:39 PDT

Created attachment 53287 [details] patch 2 (refactored, still needs expects file)

Alexey Proskuryakov

Comment 5 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.

Jonathan Dixon

Comment 6 2010-04-13 23:49:03 PDT

Created attachment 53321 [details] Patch 3 - good to go?

Adam Barth

Comment 7 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"],

Adam Barth

Comment 8 2010-04-15 01:15:02 PDT

Committed r57636: <http://trac.webkit.org/changeset/57636>

Adam Barth

Comment 9 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!

Note You need to log in before you can comment on or make changes to this bug.