[webkit-reviews] review denied: [Bug 18056] Cannot convert DOM exception prototype object to a string : [Attachment 39038] Patch v1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Cameron McCormack <cam at mcc.id.au>'s
request for review:
Bug 18056: Cannot convert DOM exception prototype object to a string
https://bugs.webkit.org/show_bug.cgi?id=18056

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +		   if ($function->signature->name eq "toString") {
> +		     $implIncludes{"PlatformString.h"} = 1;
> +		     push(@implContent, "    if
(thisValue.inherits(&${className}Prototype::s_info))\n");
> +		     push(@implContent, "	 return
jsNontrivialString(exec, \"[object \" +
String(thisValue.toThisObject(exec)->className()) + \"]\");\n");
> +		   }

Should be indented four spaces, not two.

If you use String() here you'll get the inefficient WebCore::String, which
reallocates on every append. Then convert into a UString since that's what the
jsNontrivialString function takes.

You'll want to use UString() instead. It may need to be written as
JSC::UString.

> +PASS String(new WebKitCSSMatrix().__proto__) is "[object
WebKitCSSMatrixPrototype]"
> +PASS Object.prototype.toString.call(new WebKitCSSMatrix().__proto__) is
"[object WebKitCSSMatrixPrototype]"

Doesn't WebKitCSSMatrix.prototype work? If not, is that a bug?

In some cases, I think this is the only way our internal class names are
exposed. I'm not sure that names such as DOMException, DOMWindow, DOMSelection
are suitable to be seen by the web pages. Are those names the same as in other
browsers? 

Otherwise, this looks good.

review- because of the UString issue.