Bug 3743 - Incorrect error message given for certain calls
Summary: Incorrect error message given for certain calls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P2 Minor
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-27 18:34 PDT by Oliver Hunt
Modified: 2005-06-29 15:36 PDT (History)
0 users

See Also:


Attachments
webkit layout test for this bug (925 bytes, text/html)
2005-06-27 18:36 PDT, Oliver Hunt
darin: review-
Details
expected results of testcase for layout tests (121 bytes, text/plain)
2005-06-27 18:37 PDT, Oliver Hunt
no flags Details
Preliminary patch (641 bytes, patch)
2005-06-27 18:39 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Reformatted patch (610 bytes, patch)
2005-06-28 10:36 PDT, Oliver Hunt
darin: review+
Details | Formatted Diff | Diff
Corrected webkit layout test (1.19 KB, text/html)
2005-06-28 23:12 PDT, Oliver Hunt
no flags Details
expected results of testcase for layout tests (170 bytes, text/plain)
2005-06-28 23:13 PDT, Oliver Hunt
no flags Details
Layout test, including expected output as a comment (1.41 KB, text/html)
2005-06-28 23:19 PDT, Oliver Hunt
darin: review+
Details
fast/js/toString-stack-overflow.html (1.29 KB, text/html)
2005-06-29 13:53 PDT, Geoffrey Garen
no flags Details
fast/js/toString-stack-overflow-expected.txt (590 bytes, text/plain)
2005-06-29 13:54 PDT, Geoffrey Garen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2005-06-27 18:34:07 PDT
Due to an oversight, it is possible for ObjectImp::defaultValue() to overwrite
an error thrown elsewhere in the interpreter.
Comment 1 Oliver Hunt 2005-06-27 18:36:25 PDT
Created attachment 2678 [details]
webkit layout test for this bug
Comment 2 Oliver Hunt 2005-06-27 18:37:32 PDT
Created attachment 2679 [details]
expected results of testcase for layout tests
Comment 3 Oliver Hunt 2005-06-27 18:39:12 PDT
Created attachment 2680 [details]
Preliminary patch

Still needs to be reformatted
Comment 4 Oliver Hunt 2005-06-27 18:40:50 PDT
Oops, for clarification, this bug has been pulled out from bug 3539
Comment 5 Joost de Valk (AlthA) 2005-06-27 23:26:33 PDT
Bug is confirmed. Is it dependent on the bug you pulled it from? 
Comment 6 Oliver Hunt 2005-06-27 23:38:54 PDT
It's more or less independant -- the bug comes up in bug 3539, but isn't the
cause (the exception being overwritten was the stack overflow...)
Comment 7 Oliver Hunt 2005-06-28 10:36:05 PDT
Created attachment 2689 [details]
Reformatted patch
Comment 8 Oliver Hunt 2005-06-28 10:43:28 PDT
Comment on attachment 2689 [details]
Reformatted patch

Formatting should be correct now
Comment 9 Darin Adler 2005-06-28 10:46:40 PDT
Comment on attachment 2689 [details]
Reformatted patch

The patch looks fine.

The layout test needs work, though, because it doesn't say what it's testing,
nor describe the expected results. In fact, it's actively misleading because it
talks about testing Array.toString with deep hierarchies, which is not the
point.

I'll set review+ on this, but we shouldn't land it until we have an improved
layout test.
Comment 10 Darin Adler 2005-06-28 10:46:56 PDT
Comment on attachment 2678 [details]
webkit layout test for this bug

Need a better layout test.
Comment 11 Oliver Hunt 2005-06-28 22:59:04 PDT
Yeah, oops, i'd just pulled it from bug 3539, where it was demonstrating
effectively the same thing...

fixed shortly
Comment 12 Oliver Hunt 2005-06-28 23:12:44 PDT
Created attachment 2697 [details]
Corrected webkit layout test
Comment 13 Oliver Hunt 2005-06-28 23:13:15 PDT
Created attachment 2698 [details]
expected results of testcase for layout tests
Comment 14 Oliver Hunt 2005-06-28 23:19:21 PDT
Created attachment 2699 [details]
Layout test, including expected output as a comment
Comment 15 Oliver Hunt 2005-06-29 01:16:16 PDT
Layout tests and results have been redone -- any further changes needed?
Comment 16 Oliver Hunt 2005-06-29 11:28:10 PDT
Comment on attachment 2699 [details]
Layout test, including expected output as a comment

just need someone to check whether this testcase is up to necessary standard
Comment 17 Geoffrey Garen 2005-06-29 13:53:35 PDT
Created attachment 2705 [details]
fast/js/toString-stack-overflow.html

Reformatted layout test for friendliness to manual testing
Comment 18 Geoffrey Garen 2005-06-29 13:54:46 PDT
Created attachment 2706 [details]
fast/js/toString-stack-overflow-expected.txt

Expected results for reformatted test
Comment 19 Darin Adler 2005-06-29 15:36:47 PDT
Comment on attachment 2699 [details]
Layout test, including expected output as a comment

This layout test looks fine. r=me

I think it would be even better if the expected result was mentioned in
something visible when looking at the HTML.