Bug 18056 – Cannot convert DOM exception prototype object to a string

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: DOM

[Cameron McCormack (:heycam)]

Converting a DOM exception prototype object to a string (e.g. String(DOMException.prototype)) results in an exception being thrown.  There doesn't seem to be any good reason to prevent stringification of this object, so I suggest it should evaluate to "[object DOMExceptionPrototype]" or something, like String(Node.prototype) does.

[Cameron McCormack (:heycam)]

This problem extends to all interfaces that have a toString operation defined on their interface.  For example, String(HTMLAnchorElement.prototype) also throws.

We could fix this by making CodeGeneratorJS.pm special case code generation for toString operations.

[Cameron McCormack (:heycam)]

Created attachment 39038 [details]
Patch v1

[Eric Seidel (no email)]

Comment on attachment 39038 [details]
Patch v1

I don't understand where you're putting that code.  Can you give an example of the generated output so that we know this looks right?

[Cameron McCormack (:heycam)]

(In reply to comment #3)
> I don't understand where you're putting that code.  Can you give an example of
> the generated output so that we know this looks right?

Sorry, I probably didn't include enough context in the diff.  Here's an example from JSDOMCoreException.cpp:

JSValue JSC_HOST_CALL jsDOMCoreExceptionPrototypeFunctionToString(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args)
{   
    UNUSED_PARAM(args);
    if (thisValue.inherits(&JSDOMCoreExceptionPrototype::s_info))
        return jsNontrivialString(exec, "[object " + String(thisValue.toThisObject(exec)->className()) + "]");
    if (!thisValue.inherits(&JSDOMCoreException::s_info))
        return throwError(exec, TypeError);
    JSDOMCoreException* castedThisObj = static_cast<JSDOMCoreException*>(asObject(thisValue));
    DOMCoreException* imp = static_cast<DOMCoreException*>(castedThisObj->impl());


    JSC::JSValue result = jsString(exec, imp->toString());
    return result;
}

I wondered whether I should look up the Object.prototype.toString to call instead of constructing the "[object FooPrototype]" string here, but it seemed a bit involved to do it that way.

[Eric Seidel (no email)]

Yeah, why doens't this already work?  Arent' prototypes already Objects?  so shouldn't toString() just work for them?

[Cameron McCormack (:heycam)]

It's because DOMException.prototype.toString shadows the Object.prototype.toString, and the generated code for DOMException.prototype.toString throws if its this argument is not a DOMException object -- which it isn't in this case, it's a DOMExceptionPrototype object.  So this patch makes the generated toString work on objects of the class or of the prototype class.

There are a number of interfaces that have a toString on them, causing shadowing toString properties to be placed on the interface's prototype object, most of which are tested in the test in the patch.

[Eric Seidel (no email)]

It seems we should be doing some sort of this.__proto__.toString.call() command then instead, as you suggested.  Otherwise we're just adding strange behavior in the edge-case where someone overrides Object.prototype.toString to be null or some other function.

[Eric Seidel (no email)]

There are however JS experts in WebKit who would know this stuff much better than I.

[Cameron McCormack (:heycam)]

It's probably even something Web IDL should talk about.

[Darin Adler]

Comment on attachment 39038 [details]
Patch v1

> +                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.

[Cameron McCormack (:heycam)]

(In reply to comment #10)
> > +                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.

OK.

> 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.

OK.  So I made it generate:

  return jsNontrivialString(exec,
      UString("[object ")
    + thisValue.toThisObject(exec)->className()
    + UString("]"));

since not explicitly calling UString on both char*s caused ambiguous operator+() calls.  Would it it be better to avoid the second UString conversion by doing something like:

  UString s("[object ");
  s.append(thisValue.toThisObject(exec)->className());
  s.append(']');

?

Anyway, if I do the first one above then I get linker errors:

/Developer/usr/bin/g++-4.0 -o /Users/cam/svn/WebKit-2/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore -L/Users/cam/svn/WebKit-2/WebKitBuild/Debug -F/Users/cam/svn/WebKit-2/WebKitBuild/Debug -F/System/Library/Frameworks/Carbon.framework/Frameworks -F/System/Library/Frameworks/ApplicationServices.framework/Frameworks -filelist /Users/cam/svn/WebKit-2/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/i386/WebCore.LinkFileList -framework ApplicationServices -framework Carbon -framework Cocoa -framework JavaScriptCore -licucore -lobjc -lxml2 -framework QuartzCore -framework SystemConfiguration -framework OpenGL -arch i386 -exported_symbols_list /Users/cam/svn/WebKit-2/WebKitBuild/Debug/DerivedSources/WebCore/WebCore.exp -Wl,-single_module -compatibility_version 1 -current_version 532.0 -install_name /Users/cam/svn/WebKit-2/WebKitBuild/Debug/WebCore.framework/Versions/A/WebCore -Wl,-Y,1455 -dynamiclib -mmacosx-version-min=10.4 -lWebCoreSQLite3 -lobjc -sub_library libobjc -umbrella WebKit
ld: Undefined symbols:
__ZN3JSC11concatenateEPNS_7UString3RepES2_
__ZN3JSC7UString7nullRepEv

Any hints?

> > +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?

I got the impression that because WebKitCSSMatrix didn't have [GeneratorConstructor] on it that window.WebKitCSSMatrix wouldn't exist, but that doesn't seem to be true.  I'll just use WebKitCSSMatrix.prototype.

> 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? 

They can still be exposed if you do Object.prototype.toString.call(DOMException.prototype), etc., for example.  Similarly for the actual host object instances.  So I don't think we're exposing anything new.

DOMException seems suitable as a name to expose, since it's the name of the interface in the DOM Core spec, and other browsers use that name.  For the window object, Gecko uses "Window" and "Selection" rather than "DOMWindow" and "DOMSelection".  While it's probably good to have consistently exposed [[Class]] names across browsers, that seems like a separate issue.  I don't believe Selection is part of any spec at the moment.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.

[Ahmad Saleem]

By taking the test case via Wayback Archive:

Link - https://web.archive.org/web/20130102220110/http://mcc.id.au/temp/2008/exception-prototype-stringification.html

I get following across browsers:

*** Safari 15.6 on macOS 12.5 ***

typeof DOMException.prototype == object
String(DOMException.prototype) == TypeError: The DOMException.name getter can only be used on instances of DOMException

*** Firefox Nightly 105 ***

typeof DOMException.prototype == object
String(DOMException.prototype) == TypeError: 'get name' called on an object that does not implement interface DOMException. 

*** Chrome Canary 106 ***

typeof DOMException.prototype == object
String(DOMException.prototype) == TypeError: Illegal invocation


_______

I am not sure on web-spec but just wanted to share updated test results. Please mark this bug accordingly. Thanks!

[Ryosuke Niwa]

This is config changed now that all browsers match.