Bug 130103

Summary: Improve dom error messages
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Oliver Hunt <oliver>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, cdumez, cgarcia, commit-queue, darin, eric.carlson, glenn, jer.noble, jsbell, kling, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch kling: review+

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>