Bug 5206 - Parse errors report incorrect line numbers, or none at all
Summary: Parse errors report incorrect line numbers, or none at all
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2005-09-30 13:21 PDT by Geoffrey Garen
Modified: 2013-05-08 09:32 PDT (History)
6 users (show)

See Also:


Attachments
Test1 (44 bytes, text/javascript)
2005-09-30 13:22 PDT, Geoffrey Garen
no flags Details
Test1 (82 bytes, text/html)
2005-09-30 13:22 PDT, Geoffrey Garen
no flags Details
Test2 (21 bytes, text/javascript)
2005-09-30 13:23 PDT, Geoffrey Garen
no flags Details
Test2 (83 bytes, text/html)
2005-09-30 13:24 PDT, Geoffrey Garen
no flags Details
Test 1 (corrected) (94 bytes, text/html)
2008-05-02 14:20 PDT, David Kilzer (:ddkilzer)
no flags Details
Test 2 (corrected) (94 bytes, text/html)
2008-05-02 14:21 PDT, David Kilzer (:ddkilzer)
no flags Details
Add Tests Patch v1 (4.84 KB, patch)
2012-06-19 17:00 PDT, David Kilzer (:ddkilzer)
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (533.78 KB, application/zip)
2012-06-19 20:28 PDT, WebKit Review Bot
no flags Details
Add Tests Patch v2 (6.27 KB, patch)
2012-06-20 09:39 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2005-09-30 13:21:52 PDT
I've attached two testcases, which both fail on TOT.

One problem is that syntax errors return with a line number that's relative to the current parsing unit 
instead of the current file.

One step toward fixing this would be to change the && test to an || test in setExceptionDetailsIfNeeded. 
That would cause exception information for syntax errors to be (re)set by the enclosing node.

But that really just begs the question of why we set the information incorrectly to begin with. So I think a 
better solution would be to change our code so that it only sets the information if it has valid data with 
which to do so.
Comment 1 Geoffrey Garen 2005-09-30 13:22:38 PDT
Created attachment 4115 [details]
Test1
Comment 2 Geoffrey Garen 2005-09-30 13:22:58 PDT
Created attachment 4116 [details]
Test1
Comment 3 Geoffrey Garen 2005-09-30 13:23:49 PDT
Created attachment 4117 [details]
Test2
Comment 4 Geoffrey Garen 2005-09-30 13:24:06 PDT
Created attachment 4118 [details]
Test2
Comment 5 David Kilzer (:ddkilzer) 2008-05-02 14:20:16 PDT
Created attachment 20930 [details]
Test 1 (corrected)
Comment 6 David Kilzer (:ddkilzer) 2008-05-02 14:21:09 PDT
Created attachment 20931 [details]
Test 2 (corrected)
Comment 7 David Kilzer (:ddkilzer) 2008-05-02 14:24:07 PDT
Still happens with WebKit nightly build r32778 on Mac OS X 10.5.x Leopard with Safari 3.1.1.

Comment 8 David Kilzer (:ddkilzer) 2008-05-02 14:26:01 PDT
<rdar://problem/5907896>
Comment 9 Pam Greene (IRC:pamg) 2008-12-02 16:59:20 PST
A layout test for what looks like the same issue is proposed in bug 22608, and will need new expected results when this is fixed.
Comment 10 David Kilzer (:ddkilzer) 2012-06-19 13:45:49 PDT
(In reply to comment #5)
> Created an attachment (id=20930) [details]
> Test 1 (corrected)

This test case still does not report a line number with WebKit nightly build r120705, so it's not fixed yet.

(In reply to comment #6)
> Created an attachment (id=20931) [details]
> Test 2 (corrected)

This test case reports a correct file and line number as of WebKit nightly build r120705, so it is fixed.  (Likely fixed by Bug 62613.)
Comment 11 David Kilzer (:ddkilzer) 2012-06-19 13:52:56 PDT
Note that <rdar://problem/5907896> was used to represent the fix for Bug 62613, so I will file a new radar for the remaining test case that's not fixed:  Test 1 (corrected) (Attachment 20930 [details]).
Comment 12 David Kilzer (:ddkilzer) 2012-06-19 17:00:17 PDT
Created attachment 148462 [details]
Add Tests Patch v1
Comment 13 WebKit Review Bot 2012-06-19 20:28:40 PDT
Comment on attachment 148462 [details]
Add Tests Patch v1

Attachment 148462 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12998097

New failing tests:
fast/js/parse-error-external-script-in-new-Function.html
fast/js/parse-error-external-script-in-eval.html
Comment 14 WebKit Review Bot 2012-06-19 20:28:44 PDT
Created attachment 148493 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 15 David Kilzer (:ddkilzer) 2012-06-20 09:39:01 PDT
Created attachment 148585 [details]
Add Tests Patch v2
Comment 16 Eric Seidel (no email) 2012-08-01 15:48:06 PDT
Comment on attachment 148585 [details]
Add Tests Patch v2

LGTM.
Comment 17 Eric Seidel (no email) 2013-01-04 00:47:56 PST
Looks like this got forgotten?
Comment 18 WebKit Commit Bot 2013-05-08 09:32:48 PDT
Comment on attachment 148585 [details]
Add Tests Patch v2

Clearing flags on attachment: 148585

Committed r149743: <http://trac.webkit.org/changeset/149743>
Comment 19 WebKit Commit Bot 2013-05-08 09:32:51 PDT
All reviewed patches have been landed.  Closing bug.