[webkit-reviews] Fix to Recent KJS_CHECK_EXCEPTION Change

[Maciej Stachowiak]

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  
> possible.

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.

Regards,
Maciej