Currently, many error messages include a textual representation of the expression being evaluated. This doesn't happen with SquirrelFish, which is the cause of the vast majority of the layout test failures. We currently intercept thrown exceptions and add line and sourceURL information, which is not used when printing the exception. Maciej suggested that we use this information when printing the exception and add column information as well. This will require tracking columns in the parser and storing them in the nodes.
This is causing the following failures: dom/* (116 of them) editing/selection/contenteditable-click-inside.html editing/selection/contenteditable-click-outside.html fast/forms/selected-index-assert.html fast/xpath/nsresolver-exception.xhtml platform/mac/fast/AppleScript/001.html tables/mozilla_expected_failures/core/captions1.html
I'm assigning this to myself.
The error message that was popular in the dom/* tests was "TypeError: Value undefined (result of expression acronymElem.setIdAttributeNS) is not object." This happens when converting a value to object, usually for the sake of a function call. Trunk provides many other sorts of contextual decompilation, though. The best way to see them all is probably to peruse nodes.cpp.
<rdar://problem/5916152>
Not a SquirrelFishBlocker, since we can land on trunk without great exception messages.
For reference, Firefox says things like that : >>> document.querySelectorAll('a').forEach() TypeError: document.querySelectorAll is not a function >>> document.getElementsByTagName('a').forEach() TypeError: document.getElementsByTagName("a").forEach is not a function while WebKit r34271 says: document.querySelectorAll('a').forEach() TypeError: Value undefined does not allow function calls.
Created attachment 22359 [details] Patch o' doom Here we go, error messages that Do Not Suck (tm) A few issues -- I haven't yet fixed all the error messages so some, particularly syntax errors, still do not produce awe inspiring messages, but hey have all the information required to do so. All the standard errors are now good and wonderful, and all have the additional character position information attached. The error messages are currently of the form > undefined.b++ TypeError: Result of expression 'undefined' [undefined] is not an object. > undefined[0] TypeError: Result of expression 'undefined' [undefined] is not an object. > undefined() TypeError: Result of expression 'undefined' [undefined] is not a function. > new undefined() TypeError: Result of expression 'undefined' [undefined] is not a constructor. > 1 in undefined TypeError: Result of expression 'undefined' [undefined] is not a valid argument for 'in'. > 1 instanceof undefined TypeError: Result of expression 'undefined' [undefined] is not a valid argument for 'instanceof'. >
OH, and this effects a nmber of layout tests that are looking at what the error messages are, however until i get the go ahead on the current template i won't update the test results.
Additional foo: > var foo undefined > foo.b++ TypeError: Result of expression 'foo' [undefined] is not an object. > ++foo.b TypeError: Result of expression 'foo' [undefined] is not an object.
> "someString"() TypeError: Result of expression '"someString"' [someString] is not a function. > new "someString"() TypeError: Result of expression '"someString"' [someString] is not a constructor. >
I'll review this.
Comment on attachment 22359 [details] Patch o' doom +int CodeBlock::lineNumberForVPC(const Instruction* vPC) { The opening brace should be on the next line. + * kjs/SourceRange.h: This is in the wrong position of the header list. + divot = 0; // Overflow has occurred, we can only give line number info for errors for this region Might as well put the comment on its own line(s) like all the others in this if-else statement. + JSObject* exception = Error::create(exec, TypeError, createErrorMessage(exec, codeBlock, line, divotPoint, divotPoint + endOffset, value, message), line, codeBlock->ownerNode->sourceId(), codeBlock->ownerNode->sourceURL()); Maybe it would be better to break off the createErrorMessage() call into its own line, because this line is already quite long. This goes for the other creations of errors as well. + int32_t m_startOffset; + I don't think you want the extra line here. Other than that, it looks good, so r=me with those changes. Do you have any tests for your new exception messages and line/column info?
Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/JavaScriptCore.exp M JavaScriptCore/VM/CodeBlock.cpp M JavaScriptCore/VM/CodeBlock.h M JavaScriptCore/VM/CodeGenerator.cpp M JavaScriptCore/VM/CodeGenerator.h M JavaScriptCore/VM/ExceptionHelpers.cpp M JavaScriptCore/VM/ExceptionHelpers.h M JavaScriptCore/VM/Machine.cpp M JavaScriptCore/VM/Machine.h M JavaScriptCore/kjs/DebuggerCallFrame.cpp M JavaScriptCore/kjs/Error.cpp M JavaScriptCore/kjs/Error.h M JavaScriptCore/kjs/JSGlobalObjectFunctions.cpp M JavaScriptCore/kjs/JSImmediate.cpp M JavaScriptCore/kjs/JSNotAnObject.h M JavaScriptCore/kjs/JSObject.h M JavaScriptCore/kjs/Parser.h M JavaScriptCore/kjs/SourceRange.h M JavaScriptCore/kjs/grammar.y M JavaScriptCore/kjs/lexer.cpp M JavaScriptCore/kjs/lexer.h M JavaScriptCore/kjs/nodes.cpp M JavaScriptCore/kjs/nodes.h M LayoutTests/ChangeLog M LayoutTests/fast/css/resources/font-face-descriptor-multiple-values-parsing.js M LayoutTests/fast/dom/SelectorAPI/dumpNodeList-expected.txt M LayoutTests/fast/forms/select-namedItem-expected.txt A LayoutTests/fast/js/exception-expression-offset.html A LayoutTests/fast/js/resources/exception-expression-offset.js M LayoutTests/fast/xsl/transform-xhr-doc-expected.txt M LayoutTests/http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write-expected.txt M LayoutTests/http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expected.txt M LayoutTests/http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt M LayoutTests/http/tests/security/cross-frame-access-call.html M LayoutTests/platform/mac/fast/events/updateLayoutForHitTest-expected.txt M LayoutTests/platform/mac/svg/custom/createelement-expected.txt M LayoutTests/platform/mac/tables/mozilla/bugs/bug53690-1-expected.txt M LayoutTests/platform/mac/tables/mozilla_expected_failures/bugs/bug92868_1-expected.txt Committed r35245