Closed Bug 1060873 Opened 10 years ago Closed 10 years ago

Object.isExtensible() should return false when given primitive values as input

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: 446240525, Assigned: 446240525)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])

Attachments

(1 file, 3 obsolete files)

No description provided.
Blocks: 1060870
js> Object.isExtensible("foo") TypeError: "foo" is not an object // should return false in ES6
(In reply to ziyunfei from comment #0) > I pressed the enter key by mistake.
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Attached patch bug1060873.patch (obsolete) — Splinter Review
Attachment #8481879 - Flags: review?(till)
Comment on attachment 8481879 [details] [diff] [review] bug1060873.patch Review of attachment 8481879 [details] [diff] [review]: ----------------------------------------------------------------- Great, the change looks good. It requires a few more test changes, though, as there are existing tests that now fail. Going forward, it'd be great if you could run the jstest and jit-test suites before asking for a review. See [1] for more info on that. thanks again, these changes are much appreciated! [1]: https://wiki.mozilla.org/JavaScript:New_to_SpiderMonkey#Testing_your_changes
Attachment #8481879 - Flags: review?(till)
(In reply to Till Schneidereit [:till] from comment #5) > Great, the change looks good. It requires a few more test changes, though, > as there are existing tests that now fail. Hi till, could you tell me which test(s) failed for you? Cause I did run the test suites and only saw: REGRESSIONS js1_5/extensions/toLocaleFormat-01.js js1_5/extensions/toLocaleFormat-02.js FAIL
(In reply to ziyunfei from comment #6) > Hi till, could you tell me which test(s) failed for you? Cause I did run the > test suites and only saw: > > REGRESSIONS > js1_5/extensions/toLocaleFormat-01.js > js1_5/extensions/toLocaleFormat-02.js > FAIL Interestingly, one of the test failures I saw is entirely unrelated to your patch, but happens on my machine only. I'm investigating this. The other was in fact your newly-added test. It uses a function `assertFalse`, which doesn't seem to exist. Can you change the tests to use `assertEq(Object.isExtensible(), false)`? Also, symbols are disabled for Firefox 33 and might not make Firefox 34, either. Can you put the Symbol-related test into an if block along the lines of `if (typeof Symbol !== "undefined") { [test here] }`?
Attached patch bug1060873 v2.patch (obsolete) — Splinter Review
Attachment #8481879 - Attachment is obsolete: true
Attachment #8482663 - Flags: review?(till)
Attached patch bug1060873 v3.patch (obsolete) — Splinter Review
Attachment #8482663 - Attachment is obsolete: true
Attachment #8482663 - Flags: review?(till)
Attachment #8482665 - Flags: review?(till)
Comment on attachment 8482665 [details] [diff] [review] bug1060873 v3.patch Review of attachment 8482665 [details] [diff] [review]: ----------------------------------------------------------------- Now the test isn't running properly at all anymore, because you introduced various syntax errors. Please make sure the final version you attach successfully runs.
Attachment #8482665 - Flags: review?(till)
Attachment #8482665 - Attachment is obsolete: true
Attachment #8482713 - Flags: review?(till)
(In reply to Till Schneidereit [:till] from comment #10) > Comment on attachment 8482665 [details] [diff] [review] > bug1060873 v3.patch > > Review of attachment 8482665 [details] [diff] [review]: > ----------------------------------------------------------------- > > Now the test isn't running properly at all anymore, because you introduced > various syntax errors. Please make sure the final version you attach > successfully runs. Sorry, it's my bad. I promise I will be more careful next time. Thanks for your patience!
Comment on attachment 8482713 [details] [diff] [review] bug1060873 v4.patch Review of attachment 8482713 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks for the patch and bearing with me on the change requests. remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e75a5c333c8
Attachment #8482713 - Flags: review?(till) → review+
Assignee: nobody → 446240525
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: