[Webkit-unassigned] [Bug 50773] CORS origin header not set on GET when content type request header is set

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50773


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81216|review?                     |review-
               Flag|                            |




--- Comment #13 from David Levin <levin at chromium.org>  2011-02-04 11:00:20 PST ---
(From update of attachment 81216)
View in context: https://bugs.webkit.org/attachment.cgi?id=81216&action=review

This looks good to me. I just have a few minor suggestions to clean up and then we can get this committed.

> Source/WebCore/ChangeLog:5
> +        Bug 50773: CORS origin header not set on GET when a preflight request

Ideally this is formatted more like other ChangeLog entries:

Bug title
Bug link

(on separate lines).

> Source/WebCore/loader/DocumentThreadableLoader.cpp:301
> +    // Explicitly set the origin of this request

Add "." to end of sentence.

> LayoutTests/http/tests/xmlhttprequest/cross-origin-preflight-get.html:3
> +<p>Test case for issue #50773 - the "Origin" header should be properly sent with a non-simple cross-origin resource sharing request that uses the GET method.</p>

No need to mention the bug, so you can omit "Test case for issue #50773 -"

> LayoutTests/http/tests/xmlhttprequest/cross-origin-preflight-get.html:20
> +    xhr.setRequestHeader("X-Proprietary-Header", "foo"); // make this a non-simple CORS request

Please make the comment a proper sentence.

"Make this a non-simple CORS request."

> LayoutTests/http/tests/xmlhttprequest/resources/cross-origin-preflight-get.php:2
> +// Test case for the preflight cross-origin request using GET (issue #50773)

You don't need to reference the bug number. If someone needs to find it, they can look at when the test was added and see the commit message, ChangeLog, etc. which will all reference the bug.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.