Closed
Bug 1420101
Opened 7 years ago
Closed 7 years ago
Array.prototype.values() does not exist
Categories
(Core :: JavaScript Engine, defect, P5)
Tracking
()
RESOLVED
FIXED
mozilla60
People
(Reporter: neil.kronlage, Assigned: evilpies)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file, 2 obsolete files)
8.15 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171102181127
Steps to reproduce:
[1, 2].values()
Actual results:
[1, 2].values()
TypeError: [1, 2].values is not a function
[Learn More]
Expected results:
Returned an array iterator
Reporter | ||
Comment 1•7 years ago
|
||
This appears to have been disabled due to https://bugzilla.mozilla.org/show_bug.cgi?id=1299593
I'd like to use this feature, but couldn't find any bugs tracking re-enabling it so I've opened this one.
Updated•7 years ago
|
Comment 2•7 years ago
|
||
Hmm. Well, it's been another 12 months. According to MDN, Edge and Safari are shipping Array#values now.
Then again, we've enabled this (at least) twice and been burned each time. What do you think, Tom?
Flags: needinfo?(evilpies)
Comment 3•7 years ago
|
||
In the mean time, Neil, you can work around it using this:
if (!("values" in Array.prototype)) {
Object.defineProperty(Array.prototype, "values",
Object.getOwnPropertyDescriptor(Array.prototype, Symbol.iterator));
}
(I think that'll work in the other browsers, too, not just Firefox.)
Updated•7 years ago
|
status-firefox59:
--- → fix-optional
Priority: -- → P5
We can try this again, but I would definitely like to coordinate with v8 / chrome.
Flags: needinfo?(evilpies)
Comment 5•7 years ago
|
||
(In reply to Tom Schuster [:evilpie] from comment #4)
> We can try this again, but I would definitely like to coordinate with v8 /
> chrome.
Adam, do you have any plans around this?
Flags: needinfo?(adamk)
Already got a reply via twitter https://twitter.com/_gsathya/status/937353960242814976
Flags: needinfo?(adamk)
Comment 7•7 years ago
|
||
If Sathya's up for doing the V8/Chrome-side work, I'm supportive of a coordinated effort (I've assigned the V8 tracking issue, https://bugs.chromium.org/p/v8/issues/detail?id=4247, to him).
What did you have in mind for coordination, though? The problems we've run into in the past were due to enterprise software stuck at some old version. Is your thought to just tell bug-filers "please update your software, the standard changed"? I'm worried that that isn't a very satisfying answer. Or is the idea to see if the ecosystem has moved enough that we won't get reports of breakage?
Flags: needinfo?(evilpies)
I think landing at the same time would be useful to find breakage better. And if the amount of breakage is acceptable, being broken in all browsers might be a motivator *hint*
Flags: needinfo?(evilpies)
Blink tries it again
Intent to Ship: Array.prototype.values
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yX4U__z67xo
Updated•7 years ago
|
Keywords: dev-doc-needed,
site-compat
Assignee | ||
Comment 10•7 years ago
|
||
Assignee | ||
Comment 11•7 years ago
|
||
I added a new preference javascript.options.disable_array_prototype_values. Instead of conditionally adding the property to Array.prototype, we just delete it when we don't want it. This should also work better with JSXrays.
Attachment #8947916 -
Flags: feedback?(jdemooij)
Comment 12•7 years ago
|
||
Comment on attachment 8947916 [details] [diff] [review]
Add a pref to disable Array.prototype.values
Review of attachment 8947916 [details] [diff] [review]:
-----------------------------------------------------------------
Makes sense.
Maybe also add the pref to the lists in dom/ipc/ContentPrefs.cpp and modules/libpref/init/all.js.
What about workers? Maybe we are less worried about problems there but it might be nice to set a global flag so we get consistent behavior.
Attachment #8947916 -
Flags: feedback?(jdemooij) → feedback+
Assignee | ||
Comment 13•7 years ago
|
||
Now that I added the pref to all.js, I also changed it to be default enabled, which matches most of our other preferences.
Attachment #8947916 -
Attachment is obsolete: true
Attachment #8948477 -
Flags: review?(jdemooij)
Attachment #8948477 -
Flags: review?(bzbarsky)
Comment 14•7 years ago
|
||
Comment on attachment 8948477 [details] [diff] [review]
Add default enabled pref for Array.prototype.values
Does DeleteProperty not badly deoptimize accesses on the object anymore?
r=me if so.
Attachment #8948477 -
Flags: review?(bzbarsky) → review+
Comment 15•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #14)
> Does DeleteProperty not badly deoptimize accesses on the object anymore?
DeleteProperty will convert to dictionary mode if the property is not the last one. Maybe we can make sure it's the last property added to Array.prototype? Dictionary mode is probably not too bad here, but if we can easily avoid it it would be nice.
Do you see any perf differences on shell benchmarks?
Flags: needinfo?(evilpies)
Assignee | ||
Comment 16•7 years ago
|
||
Changed the code to only conditionally add "values" to the Array.prototype object. It's not perfect, because we can't tell the different between an Array instance and Array.prototype, but we don't call JS_DefineFunctions with some arbitrary object and functionspec combination.
I also removed the nightly restriction on "values" from test_xrayToJS.xul.
Attachment #8948477 -
Attachment is obsolete: true
Attachment #8948477 -
Flags: review?(jdemooij)
Flags: needinfo?(evilpies)
Attachment #8949071 -
Flags: review?(jdemooij)
Comment 17•7 years ago
|
||
What if we move [1] before [2] to ensure "constructor" isn't added as the last property? It doesn't look like we have any strict rules when to call LinkConstructorAndPrototype when comparing other built-ins, so hopefully reordering the "constructor" property doesn't break other code...
[1] https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/js/src/vm/GlobalObject.cpp#251-253
[2] https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/js/src/vm/GlobalObject.cpp#230
Comment 18•7 years ago
|
||
Comment on attachment 8949071 [details] [diff] [review]
v2 - Add default enabled pref for Array.prototype.values
Review of attachment 8949071 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jsobj.cpp
@@ +2967,4 @@
> if (!PropertySpecNameToId(cx, fs->name, &id))
> return false;
>
> + if (StandardProtoKeyOrNull(obj) == JSProto_Array && id == NameToId(cx->names().values)) {
This is fine. I was going to say another option is to strcmp the self-hosted function name, but that would break @@iterator :)
Attachment #8949071 -
Flags: review?(jdemooij) → review+
Comment 19•7 years ago
|
||
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ca344ca6b5
Add default enabled pref for Array.prototype.values. r=jandem,bz
Comment 20•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 21•7 years ago
|
||
Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/array-prototype-values-is-now-enabled-again/
Comment 22•7 years ago
|
||
https://github.com/mdn/browser-compat-data/pull/1014
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
https://developer.mozilla.org/en-US/Firefox/Releases/60#JavaScript
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•