| Summary: | Incorrect error message given for certain calls | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Maciej Stachowiak <mjs> | ||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||
| Severity: | Minor | ||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||
| Version: | 412 | ||||||||||||||||||||||
| Hardware: | Mac | ||||||||||||||||||||||
| OS: | OS X 10.4 | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Oliver Hunt
2005-06-27 18:34:07 PDT
Created attachment 2678 [details]
webkit layout test for this bug
Created attachment 2679 [details]
expected results of testcase for layout tests
Created attachment 2680 [details]
Preliminary patch
Still needs to be reformatted
Oops, for clarification, this bug has been pulled out from bug 3539 Bug is confirmed. Is it dependent on the bug you pulled it from? 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...) Created attachment 2689 [details]
Reformatted patch
Comment on attachment 2689 [details]
Reformatted patch
Formatting should be correct now
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 on attachment 2678 [details]
webkit layout test for this bug
Need a better layout test.
Yeah, oops, i'd just pulled it from bug 3539, where it was demonstrating effectively the same thing... fixed shortly Created attachment 2697 [details]
Corrected webkit layout test
Created attachment 2698 [details]
expected results of testcase for layout tests
Created attachment 2699 [details]
Layout test, including expected output as a comment
Layout tests and results have been redone -- any further changes needed? 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
Created attachment 2705 [details]
fast/js/toString-stack-overflow.html
Reformatted layout test for friendliness to manual testing
Created attachment 2706 [details]
fast/js/toString-stack-overflow-expected.txt
Expected results for reformatted test
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.
|