[webkit-reviews] review denied: [Bug 29140] getResponseHeader returns "" instead of null as the spec requires. : [Attachment 39698] Proposed patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Carol Szabo
<carol.szabo at nokia.com>'s request for review:
Bug 29140: getResponseHeader returns "" instead of null as the spec requires.
https://bugs.webkit.org/show_bug.cgi?id=29140

Attachment 39698: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=39698&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> -    if (!isValidToken(name))
> -	   return "";
<...>
> +    //if name is not a "Valid token" it shall not match any valid header
name thus the check for isValidToken is not needed.
> +    //since httpHeaderField(name) returns a null string anyway.
>      return m_response.httpHeaderField(name);

It seems fine to remove the check - I think that we should suggest removing
this clause form XMLHttpRequest spec, too.

I do not think that the comment is helpful - it talks about code that used to
be there, but isn't any more. Also, sentences should start with capital
letters, and there should be a space after "//". I suggest just removing the
comment, and e-mailing W3C WebApps working group about changing the spec.

> +	   *
http/tests/xmlhttprequest/xmlhttprequest-invalidHeader-getRequestHeader.html:
Removed.
> +	     getResponseHeader.html now covers this case as well.

You should remove expected results for this test, as well.