Created attachment 112636 [details] test case Clicking <button onclick="for(;)">click</button> doesn't result in window.onerror being caught. This works in Firefox, and per my understanding, catching error in inline scripts like this is the primary reason for window.onerror existence.
Interesting, both test cases work fine in Chrome 14.0.835.202 but not in Safari 5.1.1 Of cause, uncaught errors in inline scripts should be reported as all others.
Created attachment 113021 [details] Patch
The problem only reproduces on syntax errors in inline event handlers. I also added a test for exceptions(not syntax errors) in inline handlers.
Looks good to me. Geoff or Sam would be the best reviewers.
Comment on attachment 113021 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113021&action=review r=me > Source/WebCore/bindings/js/JSLazyEventListener.cpp:102 > + reportCurrentException(exec); > exec->clearException(); It seems like WebCore wants an idiom where any script execution checks for and reports an exception. Long-term, it would be nice to abstract this into a common helper function / object, instead of duplicating it at each call site.
Comment on attachment 113021 [details] Patch Clearing flags on attachment: 113021 Committed r98885: <http://trac.webkit.org/changeset/98885>
All reviewed patches have been landed. Closing bug.
It broke fast/js/invalid-syntax-for-function.html on all platform: --- /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/invalid-syntax-for-function-expected.txt +++ /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/invalid-syntax-for-function-actual.txt @@ -1,2 +1,3 @@ +CONSOLE MESSAGE: line 1: SyntaxError: Invalid character: '#' This test ensures we don't crash when we are given garbage for an attribute expecting a function. https://bugs.webkit.org/show_bug.cgi?id=19025 Could you fix it please?
(In reply to comment #5) > (From update of attachment 113021 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=113021&action=review > > r=me > > > Source/WebCore/bindings/js/JSLazyEventListener.cpp:102 > > + reportCurrentException(exec); > > exec->clearException(); > > It seems like WebCore wants an idiom where any script execution checks for and reports an exception. Long-term, it would be nice to abstract this into a common helper function / object, instead of duplicating it at each call site. Absolutely agree, the code dealing with exceptions is scattered all over the bindings. In many cases there is only small subtle difference between JSC and v8 implementations and they could be refactored to have common logic(including general approach to exceptions reporting) in JS VM independent helper functions/objects or might be even basic classes for JS event listeners.
(In reply to comment #8) > It broke fast/js/invalid-syntax-for-function.html on all platform: > --- /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/invalid-syntax-for-function-expected.txt > +++ /Volumes/Data/WebKit-BuildSlave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/invalid-syntax-for-function-actual.txt > @@ -1,2 +1,3 @@ > +CONSOLE MESSAGE: line 1: SyntaxError: Invalid character: '#' > This test ensures we don't crash when we are given garbage for an attribute expecting a function. > https://bugs.webkit.org/show_bug.cgi?id=19025 > > > Could you fix it please? Done. http://trac.webkit.org/changeset/98939 Thanks for spotting this!