getOwnPropertyDescriptor proxy trap reverses the logic for new properties on non-extensible objects

RESOLVED FIXED in mozilla30

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: simon.lindholm10, Assigned: efaust)

Tracking

18 Branch
mozilla30
x86
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p2][firebug-p1])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20120921030601

Steps to reproduce:

1.
var target = {};
Object.getOwnPropertyDescriptor(new Proxy(target, {
  getOwnPropertyDescriptor: function () {
    return {value: 2, configurable: true};
   }
}), 'foo');

2.
var target = {};
Object.preventExtensions(target);
Object.getOwnPropertyDescriptor(new Proxy(target, {
  getOwnPropertyDescriptor: function () {
    return {value: 2, configurable: true};
   }
}), 'foo');


Actual results:

The first thing throws "TypeError: proxy can't report a new property on a non-extensible object", but shouldn't. The second thing should throw, but doesn't.

TrapGetOwnProperty in jsproxy.cpp does:
    // step 9
    if (target->isExtensible() && !isFixed) {
        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_REPORT_NEW);
        return false;
    }

but per spec the condition should be

    (!target->isExtensible() && !isFixed)

Updated

6 years ago
Assignee: general → ejpbruel
QA Contact: ejpbruel
Whiteboard: [js:p2]
(Reporter)

Comment 1

6 years ago
The patch for bug 795903 should also fix this.
Whiteboard: [js:p2] → [js:p2][firebug-p1]
(Reporter)

Comment 2

5 years ago
Any update on this (or bug 795903)? It breaking proxies pretty badly, and should be a simple fix.

Updated

5 years ago
Blocks: 703537

Updated

5 years ago
See Also: → bug 806299

Updated

5 years ago
See Also: → bug 795903
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
(Assignee)

Comment 3

5 years ago
Created attachment 8388745 [details] [diff] [review]
Fix

Wtf. Here's a fix.
Assignee: ejpbruel → efaustbmo
Status: NEW → ASSIGNED
Attachment #8388745 - Flags: review?(jorendorff)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Comment on attachment 8388745 [details] [diff] [review]
Fix

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

Add the test case to one of the test suites. r=me with that.

Of course proxies still need work, but thanks for knocking this one out.
Attachment #8388745 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/79d8037bf2ed
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.