Bug 41724

Summary: Improve XHR access control tests
Product: WebKit Reporter: Justin Schuh <jschuh>
Component: New BugsAssignee: Justin Schuh <jschuh>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, commit-queue, jschuh, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Justin Schuh
Reported 2010-07-06 17:17:31 PDT
Improve XHR access control tests
Attachments
Patch (15.52 KB, patch)
2010-07-06 17:42 PDT, Justin Schuh
no flags
Patch (22.08 KB, patch)
2010-07-07 17:22 PDT, Justin Schuh
no flags
Justin Schuh
Comment 1 2010-07-06 17:42:27 PDT
Justin Schuh
Comment 2 2010-07-06 20:17:03 PDT
Improving and expanding the layout tests added with the fix for bug 36483.
Adam Barth
Comment 3 2010-07-07 03:04:03 PDT
Comment on attachment 60664 [details] Patch LayoutTests/http/tests/xmlhttprequest/access-control-preflight-async-not-supported.html:10 + if (window.layoutTestController) { No braces needed. LayoutTests/http/tests/xmlhttprequest/access-control-preflight-async-not-supported.html:19 + xhr.send(""); Usually we pass null for GET (no body) instead of a body of length 0. LayoutTests/http/tests/xmlhttprequest/access-control-preflight-async-not-supported.html:56 + // Eat the exception. Why don't we log it?
Adam Barth
Comment 4 2010-07-07 03:04:41 PDT
Those comments were just drive-bys. ap should probably review your patch since he requested it.
Justin Schuh
Comment 5 2010-07-07 07:36:46 PDT
(In reply to comment #3) > (From update of attachment 60664 [details]) > LayoutTests/http/tests/xmlhttprequest/access-control-preflight-async-not-supported.html:56 > + // Eat the exception. > Why don't we log it? The exception here means the test probably succeeded (because we want the request to be denied). But I'm doing a more thorough result check in the onreadystatechange handler, which also validates that the server side matches what it should be. And I'll fix the style issues and resubmit.
Alexey Proskuryakov
Comment 6 2010-07-07 11:46:05 PDT
Comment on attachment 60664 [details] Patch Clearing review flag per the above comment. Looks good to me. Do these tests pass in Firefox? This change makes it obvious that I called "header field" a "header" at least once. I'll need to fix that eventually.
David Levin
Comment 7 2010-07-07 12:24:38 PDT
(In reply to comment #2) > Improving and expanding the layout tests added with the fix for bug 36483. I don't think that is the right bug number: "webkitpy: Move the commands folder into webkitpy/patch"
Justin Schuh
Comment 8 2010-07-07 13:24:30 PDT
(In reply to comment #7) > (In reply to comment #2) > > Improving and expanding the layout tests added with the fix for bug 36483. > > I don't think that is the right bug number: "webkitpy: Move the commands folder into webkitpy/patch" Dyslexia strikes again. Sorry, that should be bug 36843. I've finally gotten through my holiday weekend backlog so I should be posting a cleaned up patch shortly.
Justin Schuh
Comment 9 2010-07-07 17:22:52 PDT
Created attachment 60811 [details] Patch Cleaned up style issues and tweaked a few things so the tests will run in Firefox.
WebKit Commit Bot
Comment 10 2010-07-08 05:00:43 PDT
Comment on attachment 60811 [details] Patch Clearing flags on attachment: 60811 Committed r62779: <http://trac.webkit.org/changeset/62779>
WebKit Commit Bot
Comment 11 2010-07-08 05:00:49 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.