[webkit-reviews] Fix to Recent KJS_CHECK_EXCEPTION Change
mjs at apple.com
Thu Jun 9 23:01:02 PDT 2005
On Jun 9, 2005, at 8:51 PM, Darin Adler wrote:
> On Jun 9, 2005, at 7:26 AM, Mark Rowe wrote:
>> As Darin Adler mentioned in a message to webkit-dev, my recent
>> change to the behaviour of KJS_CHECK_EXCEPTION resulted in outer
>> nodes overwriting the line numbers set by inner nodes. I have
>> attached a patch that checks that the line number and source URL
>> do not exist before setting these attributes on the exception object.
> Looks great!
> Three comments:
> 1) These changes need tests. It should be straightforward to
> make tests that throw exceptions and then have exception handlers
> that check the file and line number on the caught exceptions. We
> really want to write tests whenever we fix a bug unless it's not
Oops, mea culpa on this one - when originally reviewing I assumed
this was not testable in a reasonable way, but clearly it is.
> Thanks very much for tackling this important bug. I don't mean to
> be a pain with all these review comments; keep it up!
I want to echo Darin's comment on this. Having proper line numbers
for JS exceptions is a *huge* issue for web developers debugging
their sites with Safari, and equally huge for people hacking on the
engine and trying to fix JS/DOM bugs. Both the original patch and the
follow-through are awesome.
More information about the webkit-reviews