Summary: | Improve XHR access control tests | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Schuh <jschuh> | ||||||
Component: | New Bugs | Assignee: | 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
Justin Schuh
2010-07-06 17:17:31 PDT
Created attachment 60664 [details]
Patch
Improving and expanding the layout tests added with the fix for bug 36483. 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?
Those comments were just drive-bys. ap should probably review your patch since he requested it. (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. 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.
(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" (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. Created attachment 60811 [details]
Patch
Cleaned up style issues and tweaked a few things so the tests will run in Firefox.
Comment on attachment 60811 [details] Patch Clearing flags on attachment: 60811 Committed r62779: <http://trac.webkit.org/changeset/62779> All reviewed patches have been landed. Closing bug. |