Re-align DOMException objects with what is implemented #378
Conversation
This fixes #55. As discussed there, the definition of DOMException does not match any implementations. 137856d exacerbated this, but it was already incorrect (e.g. defining own properties). Instead, implementations seem to implement DOMException as an interface with a small amount of custom bindings. This updates the specification to reflect that. As part of this, it is no longer a distinct type, but instead an interface type.
|
There are a few easy fixes we could do that IMO would make DOMException better without much burden:
But I'm aligning with the majority for now, after having gotten bitten already in trying to make DOMException nicer in the past. |
|
Some comments based on comparing to Gecko's IDL:
Thank you for doing this! |
|
@tabatkins can you help with the following Bikeshed error? I think I'm doing everything right: The <dfn dfn for="DOMException">stringification behavior</dfn> is to return the concatenation of
this {{DOMException}} object's [=DOMException/name=], ": ", and this {{DOMException}} object's
[=DOMException/message=]. |
I'll be sure to include a test case, but it looks like we can leave that out.
Good idea, let's do that instead. In that case the stringifier will be influenced by shadowing the properties; I'll be sure to test that too. In that case, @tabatkins, nevermind, no need for Bikeshed help :)
This is probably related to the grayed-out rows in https://heycam.github.io/webidl/#dfn-error-names-table. |
|
|
||
| * If an implementation gives native <emu-val>Error</emu-val> objects special powers or |
There was a problem hiding this comment.
FWIW, I've sent an email to v8-dev to discuss what we'd need to do in Chromium and there are some questions over how this would interact with https://github.com/tc39/proposal-error-stacks.
I'm not getting that error when I run Bikeshed on the file in this commit. Can you confirm that this is indeed the file causing the problem? |
|
The Bikeshed error went away when I removed the stringifier in a subsequent commit, sorry :). All good now. |
|
@bzbarsky would you mind reviewing so we can merge? |
They were removed from the WebIDL spec in 2014 ("Removed IDL exceptions,
baked in DOMException, and added Error and DOMException as types"):
whatwg/webidl@50e172e
Support for exception interfaces was implemented by treating them like
WebIDL interfaces and setting |is_exception| in IdlException (which
inherited from IdlInterface). Since DOMException was the only exception
interface we had in the tree, we can just turn it into a proper interface,
check for its name when we need to set |is_exception| and remove all the
lexer/parser/bindings scaffolding we had.
V8DOMException.{cpp,h} generated after the new DOMException.idl have been
verified to be identical to their previous version.
There is work upstream to make DOMException a proper interface (see
whatwg/webidl#378). This change can go in regardless
of the upstream pull request upstream takes, as exception interfaces have
been dead and gone for years.
Bug: 617899, 737497
Change-Id: Iea16c7da733180cd61b14471d0758d5dd68158dc
Reviewed-on: https://chromium-review.googlesource.com/558088
Reviewed-by: Kenichi Ishibashi <[email protected]>
Reviewed-by: Kentaro Hara <[email protected]>
Commit-Queue: Raphael Kubo da Costa (rakuco) <[email protected]>
Cr-Commit-Position: refs/heads/master@{#483921}
* Add tests for ways in which DOMException is unusual See whatwg/webidl#55. Follows whatwg/webidl#378.
Adapt to whatwg/webidl#378 ("Re-align DOMException objects with what is implemented"). We were reimplementing toString() in DOMException because of WebKit r29058 ("Acid3 expects ExeceptionCode constants to be defined on DOMException objects") from almost 10 years ago. A lot has happened since, and we can (and should) just use the toString() implementation from ECMAScript's %ErrorProtoype% (which is explicitly mandated to be in DOMException's inheritance chain by the WebIDL spec). Contrary to what's originally described in bug 556950, we do throw an exception when DOMException.prototype.toString() is called directly: the WebIDL spec now expects it to, and web-platform-tests/wpt#6361 tests this behavior. Additionally, we've changed the way DOMException inherits from %ErrorPrototype%: instead of doing it in V8PerContextData, we now do it in V8DOMException::installV8DOMExceptionTemplate(), as the former had problems with (i)frames detached from the DOM and toString() would just call Object.prototype.toString() instead. The only user-visible part of the change is that "toString" is no longer part of DOMException.prototype's own properties; the error message format is exactly the same in most cases (the exact steps are described in https://tc39.github.io/ecma262/#sec-error.prototype.tostring). Finally, part of http/tests/plugins/cross-frame-object-access.html's output will change from: "Error: Uncaught [object DOMException]" to "Error: Uncaught" This is tricky because it involves PPAPI and its separate process, but basically the plugin in an iframe is trying to access top.location.href, Blink is throwing a SecurityError, but the error message is sent truncated to PPAPI. The message is truncated because V8 is calling its NoSideEffectsErrorToString() when creating the message, and this one does not use the message/name accessors we install, leading to an empty message (it looked slightly better before because we the presence of our own toString() caused Object::NoSideEffectsToString() to choose a different albeit still wrong code path). Blink's handling of this is fine, as the code in V8Initializer takes care of extracting the name and error message from the DOMException V8 object that threw the exception. Bug: 556950, 737497 Change-Id: I9d81edca1de903364bb1aca5950c313885c5964a Reviewed-on: https://chromium-review.googlesource.com/558904 Commit-Queue: Raphael Kubo da Costa (rakuco) <[email protected]> Reviewed-by: Mike West <[email protected]> Reviewed-by: Yuki Shiino <[email protected]> Reviewed-by: Kentaro Hara <[email protected]> Cr-Commit-Position: refs/heads/master@{#485960}
As of whatwg/webidl#378, both new URLSearchParams(DOMException.prototype) DOMException.prototype.toString() are supposed to throw an exception due to brand checks in properties such as "name" and "message", which meant compliant implementations were always failing one of the tests here. Fix it by passing a `DOMException` instead: it has all the constants we need and passes the required property checks. Also assert that the previous behavior throws a TypeError.
This fixes #55. As discussed there, the definition of DOMException does not match any implementations. 137856d exacerbated this, but it was already incorrect (e.g. defining own properties). Instead, implementations seem to implement DOMException as an interface with a small amount of custom bindings. This updates the specification to reflect that.
As part of this, it is no longer a distinct type, but instead an interface type.
Tests: web-platform-tests/wpt#6361
Preview | Diff