[webkit-reviews] review denied: [Bug 63583] Web Inspector fails to display source for eval with syntax error : [Attachment 99162] Updated patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Juan C. Montemayor
<jmont at apple.com>'s request for review:
Bug 63583: Web Inspector fails to display source for eval with syntax error
https://bugs.webkit.org/show_bug.cgi?id=63583

Attachment 99162: Updated patch
https://bugs.webkit.org/attachment.cgi?id=99162&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=99162&action=review

This patch is ready to go, but I think it's worth fixing this one last layout
test issue.

> LayoutTests/fast/js/script-tests/eval-contained-syntax-error.js:11
> +	if (e.line == 6)
> +		successfullyParsed = true;
> +	else
> +		successfullyParsed = false;

This is kind of an abuse of how "successfullyParsed" is usually used in tests.
"successfullyParsed" is only used to indicate whether you made it to the end of
the test. To indicate success or failure of the thing you're testing, you want
something like:

debug("PASS: e.line should be 6 and is.");

AND

debug("FAIL: e.line should be 6 but instead is " + e.line + ".");