The properties "EvalError", "RangeError", "ReferenceError", "SyntaxError", "TypeError", "URIError" don't have the DontEnum attribute, and consequently they show up in a for..in statement. According to ECMA-262, chapter 15, these properties should not be enumerable ("Every other property described in this section has the attribute { DontEnum } (and no others) unless otherwise specified"). Curiously, the "Error" property does have the attribute, but the more specific errors do not.
Created attachment 38666 [details] Proposed patch
Comment on attachment 38666 [details] Proposed patch Great, but this needs a ChangeLog and a test. I expect that this will already affect other existing tests, and might not need a new one. But new one for specifically testing this might still be a good idea.
Created attachment 44310 [details] Revised patch (adds ChangeLog and test)
style-queue ran check-webkit-style on attachment 44310 [details] without any errors.
Comment on attachment 44310 [details] Revised patch (adds ChangeLog and test) cq+ on this, too. Kent, you may want to set cq? for your patches :)
Comment on attachment 44310 [details] Revised patch (adds ChangeLog and test) Rejecting patch 44310 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11730 test cases. fast/dom/prototype-inheritance.html -> failed Exiting early after 1 failures. 5574 tests run. 81.77s total testing time 5573 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 1 test case (<1%) had stderr output
Looks like this patch didn't update all the needed tests.
(In reply to comment #7) > Looks like this patch didn't update all the needed tests. Right, fast/dom/prototype-inheritance.html relies on these properties to be returned by for..in. One way to "fix" that is to remove the *Error properties from the expected test data. A different solution would be to add the set of built-in ECMA properties (that are known to be non-enumerable but present) to the set of properties being tested, thus keeping what's being tested but making the test more robust. Second option sounds obvious to me, unless there's a particular reason for that test only testing enumerable properties.
We could have an explicit list of objects instead of crawling around the DOM. Ideally such a list would be in a central place so it could be shared by all these scripts. In a perfect world it would be possible to update such a list automatically from the idls.
(In reply to comment #9) > We could have an explicit list of objects instead of crawling around the DOM. > Ideally such a list would be in a central place so it could be shared by all > these scripts. In a perfect world it would be possible to update such a list > automatically from the idls. Spun off as https://bugs.webkit.org/show_bug.cgi?id=32596 and made it a blocker for this bug. Even if that's fixed, the current patch for this bug should be changed to r-, since there's another test I need to update (one that actually tests enumerability, in which case it's correct to remove the *Error properties from the expected test output).
Comment on attachment 44310 [details] Revised patch (adds ChangeLog and test) Marking r- per Kent's comment.
Removing dependency on 32596; after reviewing the current status I've decided it's better to descope that work and focus on improving individual tests first.
Created attachment 61643 [details] Patch Requires that the fixes for https://bugs.webkit.org/show_bug.cgi?id=42258 and https://bugs.webkit.org/show_bug.cgi?id=42354 are landed first, otherwise the relevant tests would regress due to these properties no longer appearing in for..in enumeration.
Comment on attachment 61643 [details] Patch r-. Since you are changing the DontEnum status of 6 properties, we should really have tests that test all 6 properties. The existing tests only cover EvalError.
(In reply to comment #14) > (From update of attachment 61643 [details]) > r-. Since you are changing the DontEnum status of 6 properties, we should really have tests that test all 6 properties. The existing tests only cover EvalError. No, the sputnik tests cover all 6 properties, but they abort as soon as one test fails (and EvalError happens to be the first property that's tested). Have a look at e.g. LayoutTests/fast/js/sputnik/Conformance/10_Execution_Contexts/10.1_Definitions/10.1.5_Global_Object/S10.1.5_A2.1_T3.html, line 95 and onward.
Comment on attachment 61643 [details] Patch Setting r? again since the existing tests do cover all relevant properties, as described in the previous comment.
Comment on attachment 61643 [details] Patch looks ok.
Committed r63882: <http://trac.webkit.org/changeset/63882>