Summary: | test262: test262/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-44.js | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||
Component: | JavaScriptCore | Assignee: | Joseph Pecoraro <joepeck> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, joepeck, keith_miller, mark.lam, msaboff, saam | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Joseph Pecoraro
2017-04-15 17:39:36 PDT
Created attachment 307209 [details]
[PATCH] Proposed Fix
Comment on attachment 307209 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307209&action=review > LayoutTests/js/Object-getOwnPropertyNames-expected.txt:20 > +PASS getSortedOwnPropertyNames((function(){var x=new String('abc');x.bar='baz';x[999]='d';return x;})()) is ['0', '1', '2', '999', 'bar', 'length'] Unfortunately this test sorts the order, so the actual exact order I'd expect is: '0', '1', '2', '999', 'bar', 'length' I can extend this test to test the exact order. > Source/JavaScriptCore/runtime/StringObject.cpp:168 > - return JSObject::getOwnPropertyNames(thisObject, exec, propertyNames, mode); > + return JSObject::getOwnNonIndexPropertyNames(thisObject, exec, propertyNames, mode); I've always found it kind of weird that we explicitly use the use the base class name instead of just doing "Base::methodName(...)". If the base class were to ever change, all this code using the explicit name would need to change. I kept the existing style, but I'm open to making this the more explicit "Base::". Created attachment 307212 [details]
[PATCH] Proposed Fix
Rebaselined ChakraCore test results. Order improvement!
Created attachment 307214 [details]
[PATCH] Proposed Fix
Rebaselined on trunk
Comment on attachment 307214 [details]
[PATCH] Proposed Fix
LGTM
Comment on attachment 307214 [details] [PATCH] Proposed Fix Clearing flags on attachment: 307214 Committed r215400: <http://trac.webkit.org/changeset/215400> All reviewed patches have been landed. Closing bug. |