Bug 130103 - Improve dom error messages
Summary: Improve dom error messages
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-11 17:05 PDT by Oliver Hunt
Modified: 2014-03-14 13:12 PDT (History)
12 users (show)

See Also:


Attachments
Patch (224.43 KB, patch)
2014-03-11 17:07 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (239.51 KB, patch)
2014-03-14 11:08 PDT, Oliver Hunt
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2014-03-11 17:05:20 PDT
Improve dom error messages
Comment 1 Oliver Hunt 2014-03-11 17:07:23 PDT
Created attachment 226450 [details]
Patch
Comment 2 Darin Adler 2014-03-11 17:27:59 PDT
Comment on attachment 226450 [details]
Patch

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

> LayoutTests/svg/dom/SVGLengthList-basics-expected.txt:17
> +PASS text1.x.baseVal.initialize(30) threw exception TypeError: Argument 1 ('item') to impl.initialize must be an instance of SVGLength.
> +PASS text1.x.baseVal.initialize('aString') threw exception TypeError: Argument 1 ('item') to impl.initialize must be an instance of SVGLength.
> +PASS text1.x.baseVal.initialize(text1) threw exception TypeError: Argument 1 ('item') to impl.initialize must be an instance of SVGLength.

These messages don’t look so good. Where does the “impl” in “impl.initialize” come from?

> LayoutTests/svg/dom/SVGMatrix-expected.txt:97
> +PASS matrix.multiply(true) threw exception TypeError: Argument 1 ('secondMatrix') to podImpl.multiply must be an instance of SVGMatrix.
> +PASS matrix.multiply(2) threw exception TypeError: Argument 1 ('secondMatrix') to podImpl.multiply must be an instance of SVGMatrix.
> +PASS matrix.multiply('aString') threw exception TypeError: Argument 1 ('secondMatrix') to podImpl.multiply must be an instance of SVGMatrix.
> +PASS matrix.multiply(svgElement) threw exception TypeError: Argument 1 ('secondMatrix') to podImpl.multiply must be an instance of SVGMatrix.

Same question about podImpl.
Comment 3 Andreas Kling 2014-03-11 17:37:30 PDT
This looks like it will bloat our binaries. We should have a function that builds the error messages from its unique parts.
Comment 4 Oliver Hunt 2014-03-14 11:08:36 PDT
Created attachment 226735 [details]
Patch
Comment 5 Andreas Kling 2014-03-14 12:24:13 PDT
Comment on attachment 226735 [details]
Patch

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

r=me, please fix Windows build.

> Source/JavaScriptCore/runtime/Error.h:73
> +    inline EncodedJSValue throwVMTypeError(ExecState* exec, String errorMessage) { return JSValue::encode(throwTypeError(exec, errorMessage)); }

'errorMessage' will have unnecessary ref count churn here.

> Source/WebCore/bindings/js/JSDOMBinding.h:651
> +#define makeDOMBindingsTypeErrorString(...) makeDOMBindingsTypeErrorStringInternal(__VA_ARGS__, (const char*)0)

0->nullptr
Comment 6 Oliver Hunt 2014-03-14 13:12:11 PDT
Committed r165640: <http://trac.webkit.org/changeset/165640>