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+

Oliver Hunt
Reported 2014-03-11 17:05:20 PDT
Improve dom error messages
Attachments
Patch (224.43 KB, patch)
2014-03-11 17:07 PDT, Oliver Hunt
no flags
Patch (239.51 KB, patch)
2014-03-14 11:08 PDT, Oliver Hunt
kling: review+
Oliver Hunt
Comment 1 2014-03-11 17:07:23 PDT
Darin Adler
Comment 2 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.
Andreas Kling
Comment 3 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.
Oliver Hunt
Comment 4 2014-03-14 11:08:36 PDT
Andreas Kling
Comment 5 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
Oliver Hunt
Comment 6 2014-03-14 13:12:11 PDT
Note You need to log in before you can comment on or make changes to this bug.