[webkit-reviews] review denied: [Bug 120156] Calling window.close() should indicate failure with warning message : [Attachment 209639] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Vivek Galatage <vivekg at webkit.org>'s
request for review:
Bug 120156: Calling window.close() should indicate failure with warning message
https://bugs.webkit.org/show_bug.cgi?id=120156

Attachment 209639: Patch
https://bugs.webkit.org/attachment.cgi?id=209639&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=209639&action=review


> Source/WebCore/page/DOMWindow.cpp:991
> +	   String message = "Script may close only the windows those were
opened by it.";

Should wrap this literal in ASCIILiteral() for more efficient code. Not
grammatically correct. I think you mean to say:

    Scripts may close only windows that were opened by scripts.

But am not sure if this message matches the style of other console messages,
nor is it precise. I’d welcome some discussion of the best wording for the
message.

Also seems to me there is no reason to put message in a local variable; I
suggest just putting it right on the line with addMessage.