Hello, If i add a single line JavaScript comment at the end of a HTML button handler for click, the handler is not executed. I was able to reproduce the problem using the following sample: <HTML> <BODY> <input type="button" id="myButtonTest" value="Works" onClick="alert('OnClick Button1');" /> <input type="button" id="myButtonTest1" value="Doesn'tWork" onClick="alert('OnClick Button2');//;" /> </BODY> </HTML> While clicking on button "Works" displays the alert, clicking on the button "Doesn'tWork" does not bring up the alert message. Removing the //; comment or replacing it with /**/ solves the problem. I was able to reproduce the problem using: -> WebKit 41357 running on Mac Leopard -> Safari 4 both on Mac Leopard/Win -> Google Chrome on Win FF3/Opera 9.63 do not have the problem in both Mac/Win environments. Regards, Mihnea
Confirmed as a regression from 3.1.2.
Sorry, from 3.2.1
As expected, there is an unreported parsing error when attempting to parse the event handler. I'll try to use Bison's debugging to figure out why.
This regressed between r38592 and r38645. I suspect it was caused by r38635: http://trac.webkit.org/changeset/38635
This is the problem, at the top of constructFunction() in FunctionConstructor.cpp: UString program; if (args.isEmpty()) program = "(function(){})"; else if (args.size() == 1) program = "(function(){" + args.at(exec, 0).toString(exec) + "})"; else { program = "(function(" + args.at(exec, 0).toString(exec); for (size_t i = 1; i < args.size() - 1; i++) program += "," + args.at(exec, i).toString(exec); program += "){" + args.at(exec, args.size() - 1).toString(exec) + "})"; } This is possibly incorrect when anything in the middle has a single-line comment. The fix is to add newlines after substitutions. I'll assign this to myself.
Created attachment 28255 [details] Reduction If the test passes, an alert is displayed with the message PASS.
Created attachment 28258 [details] Proposed patch (without tests) Here's the fix. It needs a ChangeLog and some tests, for both the event handler and "new Function(...)" cases. I should be able to get to it by tomorrow evening.
*** Bug 24482 has been marked as a duplicate of this bug. ***
Per the duplicate, this is <rdar://problem/6663472>.
Created attachment 28450 [details] Proposed patch
Comment on attachment 28450 [details] Proposed patch r=me
Landed in r41565.