Assertion firing in JavaScriptCore/parser/parser.h for statesman.com site
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Mar 2015 23:12:39 +0000 (23:12 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Mar 2015 23:12:39 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142974

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

This patch does two things:

(1) Restore JavaScriptCore's sanitization of line and column numbers to
one-based values.

We need this because WebCore sometimes provides huge negative column
numbers.

(2) Solve the attribute event listener line numbering problem a different
way: Rather than offseting all line numbers by -1 in an attribute event
listener in order to arrange for a custom result, instead use an explicit
feature for saying "all errors in this code should map to this line number".

* bytecode/UnlinkedCodeBlock.cpp:
(JSC::UnlinkedFunctionExecutable::link):
(JSC::UnlinkedFunctionExecutable::fromGlobalCode):
* bytecode/UnlinkedCodeBlock.h:
* interpreter/Interpreter.cpp:
(JSC::StackFrame::computeLineAndColumn):
(JSC::GetStackTraceFunctor::operator()):
* interpreter/Interpreter.h:
* interpreter/StackVisitor.cpp:
(JSC::StackVisitor::Frame::computeLineAndColumn):
* parser/ParserError.h:
(JSC::ParserError::toErrorObject): Plumb through an override line number.
When a function has an override line number, all syntax and runtime
errors in the function will map to it. This is useful for attribute event
listeners.

* parser/SourceCode.h:
(JSC::SourceCode::SourceCode): Restore the old sanitization of line and
column numbers to one-based integers. It was kind of a hack to remove this.

* runtime/Executable.cpp:
(JSC::ScriptExecutable::ScriptExecutable):
(JSC::FunctionExecutable::fromGlobalCode):
* runtime/Executable.h:
(JSC::ScriptExecutable::setOverrideLineNo):
(JSC::ScriptExecutable::hasOverrideLineNo):
(JSC::ScriptExecutable::overrideLineNo):
* runtime/FunctionConstructor.cpp:
(JSC::constructFunctionSkippingEvalEnabledCheck):
* runtime/FunctionConstructor.h: Plumb through an override line number.

Source/WebCore:

* bindings/js/JSLazyEventListener.cpp:
(WebCore::JSLazyEventListener::initializeJSFunction): Use the new override
line number API to guarantee that errors will map to the .html file locations
that we like.

* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::eventHandlerPosition): Added a FIXME to cover
some cases where our line and column numbers are still nonsense.

LayoutTests:

No test covering this ASSERT because I couldn't design a way to reproduce
it after trying for a few hours. Simply loading the original ASSERTing
content from disk is not enough to reproduce this bug.

* fast/profiler/dead-time-expected.txt:
* fast/profiler/inline-event-handler-expected.txt:
* fast/profiler/stop-profiling-after-setTimeout-expected.txt: These are
progressions, where we used to get the line number wrong.

* fast/dom/attribute-event-listener-errors-expected.txt: Added.
* fast/dom/attribute-event-listener-errors.html: Added. This test covers
a subtle way in which the new mechanism for attribute event listener
line numbers is more accurate than the old one.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182034 268f45cc-cd09-0410-ab3c-d52691b4dbfc

21 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/attribute-event-listener-errors-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/attribute-event-listener-errors.html [new file with mode: 0644]
LayoutTests/fast/profiler/dead-time-expected.txt
LayoutTests/fast/profiler/inline-event-handler-expected.txt
LayoutTests/fast/profiler/stop-profiling-after-setTimeout-expected.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/interpreter/Interpreter.h
Source/JavaScriptCore/interpreter/StackVisitor.cpp
Source/JavaScriptCore/parser/ParserError.h
Source/JavaScriptCore/parser/SourceCode.h
Source/JavaScriptCore/runtime/Executable.cpp
Source/JavaScriptCore/runtime/Executable.h
Source/JavaScriptCore/runtime/FunctionConstructor.cpp
Source/JavaScriptCore/runtime/FunctionConstructor.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSLazyEventListener.cpp
Source/WebCore/bindings/js/ScriptController.cpp

index b28fda1..7742b49 100644 (file)
@@ -1,3 +1,24 @@
+2015-03-26  Geoffrey Garen  <ggaren@apple.com>
+
+        Assertion firing in JavaScriptCore/parser/parser.h for statesman.com site
+        https://bugs.webkit.org/show_bug.cgi?id=142974
+
+        Reviewed by Joseph Pecoraro.
+
+        No test covering this ASSERT because I couldn't design a way to reproduce
+        it after trying for a few hours. Simply loading the original ASSERTing
+        content from disk is not enough to reproduce this bug.
+
+        * fast/profiler/dead-time-expected.txt:
+        * fast/profiler/inline-event-handler-expected.txt:
+        * fast/profiler/stop-profiling-after-setTimeout-expected.txt: These are
+        progressions, where we used to get the line number wrong.
+
+        * fast/dom/attribute-event-listener-errors-expected.txt: Added.
+        * fast/dom/attribute-event-listener-errors.html: Added. This test covers
+        a subtle way in which the new mechanism for attribute event listener
+        line numbers is more accurate than the old one.
+
 2015-03-26  Brady Eidson  <beidson@apple.com>
 
         Apply ContentExtension actions after redirects.
diff --git a/LayoutTests/fast/dom/attribute-event-listener-errors-expected.txt b/LayoutTests/fast/dom/attribute-event-listener-errors-expected.txt
new file mode 100644 (file)
index 0000000..4236767
--- /dev/null
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: line 4: ReferenceError: Can't find variable: error
+CONSOLE MESSAGE: line 5: SyntaxError: Invalid character: '@'
+This test verifies that an attribute event listener error shows the right line number even if the attribute contains newlines.
+  
diff --git a/LayoutTests/fast/dom/attribute-event-listener-errors.html b/LayoutTests/fast/dom/attribute-event-listener-errors.html
new file mode 100644 (file)
index 0000000..f1ef943
--- /dev/null
@@ -0,0 +1,15 @@
+<body>
+       <pre>This test verifies that an attribute event listener error shows the right line number even if the attribute contains newlines.</pre>
+
+       <button id ="1" onclick="&#10;&#10;&#10;error()"></button>
+       <button id ="2" onclick="&#10;&#10;&#10;$@_."></button>
+
+<script>
+if (window.testRunner)
+       testRunner.dumpAsText();
+
+document.getElementById("1").click();
+document.getElementById("2").click();
+</script>
+
+</body>
index efdf99b..849e8a0 100644 (file)
@@ -4,7 +4,7 @@ To run this test manually, load it in the browser then load the WebInspector and
 
 Profile title: Dead time in profile.
 Thread_1 (no file) (line 0:0)
-   onload dead-time.html (line 20:52)
+   onload dead-time.html (line 21:52)
       startTest dead-time.html (line 13:1)
          setTimeout (no file) (line 0:0)
    (program) dead-time.html (line 1:1)
index ac41064..961a30f 100644 (file)
@@ -7,7 +7,7 @@ Thread_1 (no file) (line 0:0)
    startTest inline-event-handler.html (line 11:1)
       getElementById (no file) (line 0:0)
       click (no file) (line 0:0)
-         onclick inline-event-handler.html (line 30:135)
+         onclick inline-event-handler.html (line 31:135)
             eventListener inline-event-handler.html (line 17:26)
                anonymousFunction profiler-test-JS-resources.js (line 29:37)
                   insertNewText profiler-test-JS-resources.js (line 17:26)
index b618c5f..2bd2916 100644 (file)
@@ -4,7 +4,7 @@ To run this test manually, load it in the browser then load the WebInspector and
 
 Profile title: Stop profiling from a timeout
 Thread_1 (no file) (line 0:0)
-   onload stop-profiling-after-setTimeout.html (line 20:52)
+   onload stop-profiling-after-setTimeout.html (line 21:52)
       startTest stop-profiling-after-setTimeout.html (line 13:1)
          setTimeout (no file) (line 0:0)
    (program) stop-profiling-after-setTimeout.html (line 1:1)
index 88f53f1..85b4482 100644 (file)
@@ -1,3 +1,54 @@
+2015-03-26  Geoffrey Garen  <ggaren@apple.com>
+
+        Assertion firing in JavaScriptCore/parser/parser.h for statesman.com site
+        https://bugs.webkit.org/show_bug.cgi?id=142974
+
+        Reviewed by Joseph Pecoraro.
+
+        This patch does two things:
+
+        (1) Restore JavaScriptCore's sanitization of line and column numbers to
+        one-based values.
+
+        We need this because WebCore sometimes provides huge negative column
+        numbers.
+
+        (2) Solve the attribute event listener line numbering problem a different
+        way: Rather than offseting all line numbers by -1 in an attribute event
+        listener in order to arrange for a custom result, instead use an explicit
+        feature for saying "all errors in this code should map to this line number".
+
+        * bytecode/UnlinkedCodeBlock.cpp:
+        (JSC::UnlinkedFunctionExecutable::link):
+        (JSC::UnlinkedFunctionExecutable::fromGlobalCode):
+        * bytecode/UnlinkedCodeBlock.h:
+        * interpreter/Interpreter.cpp:
+        (JSC::StackFrame::computeLineAndColumn):
+        (JSC::GetStackTraceFunctor::operator()):
+        * interpreter/Interpreter.h:
+        * interpreter/StackVisitor.cpp:
+        (JSC::StackVisitor::Frame::computeLineAndColumn):
+        * parser/ParserError.h:
+        (JSC::ParserError::toErrorObject): Plumb through an override line number.
+        When a function has an override line number, all syntax and runtime
+        errors in the function will map to it. This is useful for attribute event
+        listeners.
+        * parser/SourceCode.h:
+        (JSC::SourceCode::SourceCode): Restore the old sanitization of line and
+        column numbers to one-based integers. It was kind of a hack to remove this.
+
+        * runtime/Executable.cpp:
+        (JSC::ScriptExecutable::ScriptExecutable):
+        (JSC::FunctionExecutable::fromGlobalCode):
+        * runtime/Executable.h:
+        (JSC::ScriptExecutable::setOverrideLineNo):
+        (JSC::ScriptExecutable::hasOverrideLineNo):
+        (JSC::ScriptExecutable::overrideLineNo):
+        * runtime/FunctionConstructor.cpp:
+        (JSC::constructFunctionSkippingEvalEnabledCheck):
+        * runtime/FunctionConstructor.h: Plumb through an override line number.
+
 2015-03-26  Filip Pizlo  <fpizlo@apple.com>
 
         If we're in code for accessing scoped arguments, we should probably check if the object is a scoped arguments rather than checking if it's a direct arguments.
index 15711e2..8c4cf73 100644 (file)
@@ -132,7 +132,7 @@ void UnlinkedFunctionExecutable::visitChildren(JSCell* cell, SlotVisitor& visito
     visitor.append(&thisObject->m_symbolTableForConstruct);
 }
 
-FunctionExecutable* UnlinkedFunctionExecutable::link(VM& vm, const SourceCode& ownerSource)
+FunctionExecutable* UnlinkedFunctionExecutable::link(VM& vm, const SourceCode& ownerSource, int overrideLineNo)
 {
     SourceCode source = m_sourceOverride ? SourceCode(m_sourceOverride) : ownerSource;
     unsigned firstLine = source.firstLine() + m_firstLineOffset;
@@ -145,10 +145,15 @@ FunctionExecutable* UnlinkedFunctionExecutable::link(VM& vm, const SourceCode& o
     unsigned endColumn = m_unlinkedBodyEndColumn + (endColumnIsOnStartLine ? startColumn : 1);
 
     SourceCode code(source.provider(), startOffset, startOffset + m_sourceLength, firstLine, startColumn);
-    return FunctionExecutable::create(vm, code, this, firstLine, firstLine + m_lineCount, startColumn, endColumn);
+    FunctionExecutable* result = FunctionExecutable::create(vm, code, this, firstLine, firstLine + m_lineCount, startColumn, endColumn);
+    if (overrideLineNo != -1)
+        result->setOverrideLineNo(overrideLineNo);
+    return result;
 }
 
-UnlinkedFunctionExecutable* UnlinkedFunctionExecutable::fromGlobalCode(const Identifier& name, ExecState& exec, const SourceCode& source, JSObject*& exception)
+UnlinkedFunctionExecutable* UnlinkedFunctionExecutable::fromGlobalCode(
+    const Identifier& name, ExecState& exec, const SourceCode& source, 
+    JSObject*& exception, int overrideLineNo)
 {
     ParserError error;
     VM& vm = exec.vm();
@@ -160,7 +165,7 @@ UnlinkedFunctionExecutable* UnlinkedFunctionExecutable::fromGlobalCode(const Ide
         globalObject.debugger()->sourceParsed(&exec, source.provider(), error.line(), error.message());
 
     if (error.isValid()) {
-        exception = error.toErrorObject(&globalObject, source);
+        exception = error.toErrorObject(&globalObject, source, overrideLineNo);
         return nullptr;
     }
 
index 0aa83dc..5576ccf 100644 (file)
@@ -136,9 +136,11 @@ public:
         VM&, const SourceCode&, CodeSpecializationKind, DebuggerMode, ProfilerMode, 
         ParserError&);
 
-    static UnlinkedFunctionExecutable* fromGlobalCode(const Identifier&, ExecState&, const SourceCode&, JSObject*& exception);
+    static UnlinkedFunctionExecutable* fromGlobalCode(
+        const Identifier&, ExecState&, const SourceCode&, JSObject*& exception, 
+        int overrideLineNo);
 
-    FunctionExecutable* link(VM&, const SourceCode&);
+    FunctionExecutable* link(VM&, const SourceCode&, int overrideLineNo = -1);
 
     void clearCodeForRecompilation()
     {
index bc885a6..1286728 100644 (file)
@@ -440,6 +440,9 @@ void StackFrame::computeLineAndColumn(unsigned& line, unsigned& column)
 
     line = divotLine + lineOffset;
     column = divotColumn + (divotLine ? 1 : firstLineColumnOffset);
+
+    if (executable->hasOverrideLineNo())
+        line = executable->overrideLineNo();
 }
 
 void StackFrame::expressionInfo(int& divot, int& startOffset, int& endOffset, unsigned& line, unsigned& column)
@@ -490,7 +493,7 @@ public:
                 StackFrame s = {
                     Strong<JSObject>(vm, visitor->callee()),
                     getStackFrameCodeType(visitor),
-                    Strong<ExecutableBase>(vm, codeBlock->ownerExecutable()),
+                    Strong<ScriptExecutable>(vm, codeBlock->ownerExecutable()),
                     Strong<UnlinkedCodeBlock>(vm, codeBlock->unlinkedCodeBlock()),
                     codeBlock->source(),
                     codeBlock->ownerExecutable()->lineNo(),
@@ -501,7 +504,7 @@ public:
                 };
                 m_results.append(s);
             } else {
-                StackFrame s = { Strong<JSObject>(vm, visitor->callee()), StackFrameNativeCode, Strong<ExecutableBase>(), Strong<UnlinkedCodeBlock>(), 0, 0, 0, 0, 0, String()};
+                StackFrame s = { Strong<JSObject>(vm, visitor->callee()), StackFrameNativeCode, Strong<ScriptExecutable>(), Strong<UnlinkedCodeBlock>(), 0, 0, 0, 0, 0, String()};
                 m_results.append(s);
             }
     
index 6315c4a..6bf88db 100644 (file)
@@ -81,7 +81,7 @@ namespace JSC {
     struct StackFrame {
         Strong<JSObject> callee;
         StackFrameCodeType codeType;
-        Strong<ExecutableBase> executable;
+        Strong<ScriptExecutable> executable;
         Strong<UnlinkedCodeBlock> codeBlock;
         RefPtr<SourceProvider> code;
         int lineOffset;
index 669e3b6..4d3f6e7 100644 (file)
@@ -293,6 +293,9 @@ void StackVisitor::Frame::computeLineAndColumn(unsigned& line, unsigned& column)
 
     line = divotLine + codeBlock->ownerExecutable()->lineNo();
     column = divotColumn + (divotLine ? 1 : codeBlock->firstLineColumnOffset());
+
+    if (codeBlock->ownerExecutable()->hasOverrideLineNo())
+        line = codeBlock->ownerExecutable()->overrideLineNo();
 }
 
 void StackVisitor::Frame::retrieveExpressionInfo(int& divot, int& startOffset, int& endOffset, unsigned& line, unsigned& column)
index 299984e..b156290 100644 (file)
@@ -85,13 +85,18 @@ public:
     const String& message() const { return m_message; }
     int line() const { return m_line; }
 
-    JSObject* toErrorObject(JSGlobalObject* globalObject, const SourceCode& source)
+    JSObject* toErrorObject(
+        JSGlobalObject* globalObject, const SourceCode& source, 
+        int overrideLineNo = -1)
     {
         switch (m_type) {
         case ErrorNone:
             return nullptr;
         case SyntaxError:
-            return addErrorInfo(globalObject->globalExec(), createSyntaxError(globalObject, m_message), m_line, source);
+            return addErrorInfo(
+                globalObject->globalExec(), 
+                createSyntaxError(globalObject, m_message), 
+                overrideLineNo == -1 ? m_line : overrideLineNo, source);
         case EvalError:
             return createSyntaxError(globalObject, m_message);
         case StackOverflow: {
index 792c19e..73d23db 100644 (file)
@@ -63,8 +63,8 @@ namespace JSC {
             : m_provider(provider)
             , m_startChar(0)
             , m_endChar(m_provider->source().length())
-            , m_firstLine(std::max(firstLine, 0))
-            , m_startColumn(std::max(startColumn, 0))
+            , m_firstLine(std::max(firstLine, 1))
+            , m_startColumn(std::max(startColumn, 1))
         {
         }
 
@@ -72,8 +72,8 @@ namespace JSC {
             : m_provider(provider)
             , m_startChar(start)
             , m_endChar(end)
-            , m_firstLine(std::max(firstLine, 0))
-            , m_startColumn(std::max(startColumn, 0))
+            , m_firstLine(std::max(firstLine, 1))
+            , m_startColumn(std::max(startColumn, 1))
         {
         }
 
index cbbabe0..ad6ab0f 100644 (file)
@@ -100,6 +100,7 @@ ScriptExecutable::ScriptExecutable(Structure* structure, VM& vm, const SourceCod
     , m_hasCapturedVariables(false)
     , m_neverInline(false)
     , m_didTryToEnterInLoop(false)
+    , m_overrideLineNo(-1)
     , m_firstLine(-1)
     , m_lastLine(-1)
     , m_startColumn(UINT_MAX)
@@ -608,12 +609,17 @@ void FunctionExecutable::unlinkCalls()
 #endif
 }
 
-FunctionExecutable* FunctionExecutable::fromGlobalCode(const Identifier& name, ExecState& exec, const SourceCode& source, JSObject*& exception)
+FunctionExecutable* FunctionExecutable::fromGlobalCode(
+    const Identifier& name, ExecState& exec, const SourceCode& source, 
+    JSObject*& exception, int overrideLineNo)
 {
-    UnlinkedFunctionExecutable* unlinkedExecutable = UnlinkedFunctionExecutable::fromGlobalCode(name, exec, source, exception);
+    UnlinkedFunctionExecutable* unlinkedExecutable = 
+        UnlinkedFunctionExecutable::fromGlobalCode(
+            name, exec, source, exception, overrideLineNo);
     if (!unlinkedExecutable)
         return nullptr;
-    return unlinkedExecutable->link(exec.vm(), source);
+
+    return unlinkedExecutable->link(exec.vm(), source, overrideLineNo);
 }
 
 void ExecutableBase::dump(PrintStream& out) const
index 6022791..de0a1e4 100644 (file)
@@ -358,6 +358,9 @@ public:
     intptr_t sourceID() const { return m_source.providerID(); }
     const String& sourceURL() const { return m_source.provider()->url(); }
     int lineNo() const { return m_firstLine; }
+    void setOverrideLineNo(int overrideLineNo) { m_overrideLineNo = overrideLineNo; }
+    bool hasOverrideLineNo() const { return m_overrideLineNo != -1; }
+    int overrideLineNo() const { return m_overrideLineNo; }
     int lastLine() const { return m_lastLine; }
     unsigned startColumn() const { return m_startColumn; }
     unsigned endColumn() const { return m_endColumn; }
@@ -429,6 +432,7 @@ protected:
     bool m_hasCapturedVariables;
     bool m_neverInline;
     bool m_didTryToEnterInLoop;
+    int m_overrideLineNo;
     int m_firstLine;
     int m_lastLine;
     unsigned m_startColumn;
@@ -549,7 +553,9 @@ public:
         executable->finishCreation(vm);
         return executable;
     }
-    static FunctionExecutable* fromGlobalCode(const Identifier& name, ExecState&, const SourceCode&, JSObject*& exception);
+    static FunctionExecutable* fromGlobalCode(
+        const Identifier& name, ExecState&, const SourceCode&, 
+        JSObject*& exception, int overrideLineNo);
 
     static void destroy(JSCell*);
         
index 76a7140..9ad29b0 100644 (file)
@@ -86,7 +86,10 @@ JSObject* constructFunction(ExecState* exec, JSGlobalObject* globalObject, const
     return constructFunctionSkippingEvalEnabledCheck(exec, globalObject, args, functionName, sourceURL, position);
 }
 
-JSObject* constructFunctionSkippingEvalEnabledCheck(ExecState* exec, JSGlobalObject* globalObject, const ArgList& args, const Identifier& functionName, const String& sourceURL, const TextPosition& position)
+JSObject* constructFunctionSkippingEvalEnabledCheck(
+    ExecState* exec, JSGlobalObject* globalObject, const ArgList& args, 
+    const Identifier& functionName, const String& sourceURL, 
+    const TextPosition& position, int overrideLineNo)
 {
     // How we stringify functions is sometimes important for web compatibility.
     // See https://bugs.webkit.org/show_bug.cgi?id=24350.
@@ -113,7 +116,7 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(ExecState* exec, JSGlobalObj
 
     SourceCode source = makeSource(program, sourceURL, position);
     JSObject* exception = nullptr;
-    FunctionExecutable* function = FunctionExecutable::fromGlobalCode(functionName, *exec, source, exception);
+    FunctionExecutable* function = FunctionExecutable::fromGlobalCode(functionName, *exec, source, exception, overrideLineNo);
     if (!function) {
         ASSERT(exception);
         return exec->vm().throwException(exec, exception);
index dd7a662..ec7174e 100644 (file)
@@ -59,7 +59,9 @@ private:
 JSObject* constructFunction(ExecState*, JSGlobalObject*, const ArgList&, const Identifier& functionName, const String& sourceURL, const WTF::TextPosition&);
 JSObject* constructFunction(ExecState*, JSGlobalObject*, const ArgList&);
 
-JS_EXPORT_PRIVATE JSObject* constructFunctionSkippingEvalEnabledCheck(ExecState*, JSGlobalObject*, const ArgList&, const Identifier&, const String&, const WTF::TextPosition&);
+JS_EXPORT_PRIVATE JSObject* constructFunctionSkippingEvalEnabledCheck(
+    ExecState*, JSGlobalObject*, const ArgList&, const Identifier&, 
+    const String&, const WTF::TextPosition&, int overrideLineNo = -1);
 
 } // namespace JSC
 
index 3c0f91b..821577b 100644 (file)
@@ -1,3 +1,19 @@
+2015-03-26  Geoffrey Garen  <ggaren@apple.com>
+
+        Assertion firing in JavaScriptCore/parser/parser.h for statesman.com site
+        https://bugs.webkit.org/show_bug.cgi?id=142974
+
+        Reviewed by Joseph Pecoraro.
+
+        * bindings/js/JSLazyEventListener.cpp:
+        (WebCore::JSLazyEventListener::initializeJSFunction): Use the new override
+        line number API to guarantee that errors will map to the .html file locations
+        that we like.
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::eventHandlerPosition): Added a FIXME to cover
+        some cases where our line and column numbers are still nonsense.
+
 2015-03-26  Beth Dakin  <bdakin@apple.com>
 
         Relevant repainted objects callback is inaccurate and inconsistent for PDF 
index c3ab825..6e27685 100644 (file)
@@ -24,6 +24,7 @@
 #include "Frame.h"
 #include "JSNode.h"
 #include "ScriptController.h"
+#include <runtime/Executable.h>
 #include <runtime/FunctionConstructor.h>
 #include <runtime/IdentifierInlines.h>
 #include <wtf/NeverDestroyed.h>
@@ -103,15 +104,13 @@ JSObject* JSLazyEventListener::initializeJSFunction(ScriptExecutionContext* exec
     args.append(jsNontrivialString(exec, m_eventParameterName));
     args.append(jsStringWithCache(exec, m_code));
 
-    // Move our text position backward one line. Creating an anonymous function
-    // will add a line for a function declaration, but we want our line number
-    // to match up with where the attribute was declared.
-    TextPosition position(
-        OrdinalNumber::fromOneBasedInt(
-            m_position.m_line.oneBasedInt() - 1), m_position.m_column);
+    // We want all errors to refer back to the line on which our attribute was
+    // declared, regardless of any newlines in our JavaScript source text.
+    int overrideLineNo = m_position.m_line.oneBasedInt();
+
     JSObject* jsFunction = constructFunctionSkippingEvalEnabledCheck(
         exec, exec->lexicalGlobalObject(), args, Identifier(exec, m_functionName), 
-        m_sourceURL, position);
+        m_sourceURL, m_position, overrideLineNo);
 
     if (exec->hadException()) {
         reportCurrentException(exec);
@@ -120,6 +119,7 @@ JSObject* JSLazyEventListener::initializeJSFunction(ScriptExecutionContext* exec
     }
 
     JSFunction* listenerAsFunction = jsCast<JSFunction*>(jsFunction);
+
     if (m_originalNode) {
         if (!wrapper()) {
             // Ensure that 'node' has a JavaScript wrapper to mark the event listener we're creating.
index 2804f49..969b119 100644 (file)
@@ -274,6 +274,11 @@ JSDOMWindowShell* ScriptController::initScript(DOMWrapperWorld& world)
 
 TextPosition ScriptController::eventHandlerPosition() const
 {
+    // FIXME: If we are not currently parsing, we should use our current location
+    // in JavaScript, to cover cases like "element.setAttribute('click', ...)".
+
+    // FIXME: This location maps to the end of the HTML tag, and not to the
+    // exact column number belonging to the event handler attribute.
     ScriptableDocumentParser* parser = m_frame.document()->scriptableDocumentParser();
     if (parser)
         return parser->textPosition();