[webkit-reviews] review denied: [Bug 169286] XMLHttpRequest: getAllResponseHeaders() : [Attachment 305017] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied youenn fablet <youennf at gmail.com>'s
request for review:
Bug 169286: XMLHttpRequest: getAllResponseHeaders()
https://bugs.webkit.org/show_bug.cgi?id=169286

Attachment 305017: Patch

https://bugs.webkit.org/attachment.cgi?id=305017&action=review




--- Comment #2 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 305017
  --> https://bugs.webkit.org/attachment.cgi?id=305017
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305017&action=review

> Source/WebCore/ChangeLog:3
> +	   XMLHttpRequest: getAllResponseHeaders()

We already supported it so this should explain what you changed.

> Source/WebCore/ChangeLog:9
> +

Can you please point to the spec in your changeling so we know why this is the
expected behavior?

> Source/WebCore/xml/XMLHttpRequest.cpp:930
> +

Why the extra line here?

> Source/WebCore/xml/XMLHttpRequest.cpp:938
> +	   for (const auto& header : m_response.httpHeaderFields()) {

We usually omit the const here.

> Source/WebCore/xml/XMLHttpRequest.cpp:941
> +	       stringBuilder.append(':');

appendLiteral(": ");

> Source/WebCore/xml/XMLHttpRequest.cpp:942
> +	       stringBuilder.append(' ');

not needed.

> Source/WebCore/xml/XMLHttpRequest.cpp:944
> +	       stringBuilder.append('\r');

appendLiteral("\r\n");

> Source/WebCore/xml/XMLHttpRequest.cpp:945
> +	       stringBuilder.append('\n');

Not needed.

> LayoutTests/imported/w3c/ChangeLog:3
> +	   fix-169286

?