[es7] implement Object.getOwnPropertyDescriptors() proposal

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: andrea.giammarchi, Assigned: evilpie)

Tracking

({dev-doc-complete})

Trunk
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [DocArea=JS])

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36

Steps to reproduce:

Apologies if this is not the best channel but it's already happening in Chrome so I've thought to file a bug/request in here too. The Chromium related issue is the following one: https://codereview.chromium.org/1658773003/


Actual results:

Object.getOwnPropertyDescriptors is not defined


Expected results:

There is a stage 3 proposal waiting to be promoted to stage 4: https://github.com/tc39/proposal-object-getownpropertydescriptors#objectgetownpropertydescriptors-proposal-polyfill

Expecting Object.getOwnPropertyDescriptors to be available and return a generic object with all own properties (names + Symbols) and relative descriptors as described in the proposal.

A polyfill-like version of what should happen is described in here:
https://github.com/tc39/proposal-object-getownpropertydescriptors#proposed-solution

Thanks in advance for considering this change or for redirecting this to the right team.

Best Regards

Updated

3 years ago
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Duplicate of this bug: 1246069
(Assignee)

Updated

3 years ago
Assignee: nobody → evilpies
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 3

3 years ago
This is not yet in the current spec at http://tc39.github.io/ecma262. There is still some discussion about the case when the descriptor returned by [[GetOwnProperty]] is undefined: https://github.com/tc39/ecma262/pull/582

Comment 4

3 years ago
https://github.com/tc39/ecma262/pull/582 has now been merged unchanged. In July, we'll get committee consensus on the additional "if" check in https://github.com/tc39/ecma262/pull/593 which only applies to Proxies.
(Assignee)

Comment 5

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e402e788d6f&selectedJob=21987444
dom/plugins/test/mochitest/test_defaultValue.html is failing, I suspect we are passing a symbol to function that can't handle it. As usual, just running the test locally doesn't actually reproduce it.
(Assignee)

Updated

3 years ago
Attachment #8756410 - Flags: review?(jorendorff)
(Assignee)

Comment 6

3 years ago
I made a few changes to these tests to avoid having to import a lot of code from the test suite.
Attachment #8760090 - Flags: review?(jorendorff)
Comment on attachment 8756410 [details] [diff] [review]
Implement Object.getOwnPropertyDescriptors

Review of attachment 8756410 [details] [diff] [review]:
-----------------------------------------------------------------

Seems totally fine. Can you file a followup to track the spec change that's in the works for thi stage 4 proposal? I mean the extra if, which Jordan says only affects the proxy case of returns from [[GetOwnProperty]].
Attachment #8756410 - Flags: review?(jorendorff) → review+
Comment on attachment 8760090 [details] [diff] [review]
Port tests from tests262

Review of attachment 8760090 [details] [diff] [review]:
-----------------------------------------------------------------

Hmmm, I wonder about putting these in the test262 directory, which I don't think is ever run. Is there somewhere we can put them where they will be run for sure? add a bunch fo reportCompare() calls and put them in js/tests/ for example?

r=me with assurances that they are actually being run in automation.
Attachment #8760090 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 9

3 years ago
Going to merge this before landing.
Comment on attachment 8761660 [details] [diff] [review]
Add getOwnPropertyDescriptors to test_xray

r=me
Attachment #8761660 - Flags: review+

Comment 11

3 years ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5238e71b3635
Implement Object.getOwnPropertyDescriptors. r=efaust,bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/06a54c686fc9
Import test262 tests. r=efaust
(Assignee)

Updated

3 years ago
Blocks: 1279303

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5238e71b3635
https://hg.mozilla.org/mozilla-central/rev/06a54c686fc9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.