1420101 - Array.prototype.values() does not exist

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P5)

Description

[neil.kronlage]

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

Comment 1

[neil.kronlage]

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.

Comment 2

[Jason Orendorff [:jorendorff]]

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?

Comment 3

[Jason Orendorff [:jorendorff]]

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.)

Comment 4

[Tom S [:evilpie]]

We can try this again, but I would definitely like to coordinate with v8 / chrome.

Comment 5

[Till Schneidereit [:till]]

(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?

Comment 6

[Tom S [:evilpie]]

Already got a reply via twitter https://twitter.com/_gsathya/status/937353960242814976

Comment 7

[Adam Klein]

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?

Comment 8

[Tom S [:evilpie]]

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*

Comment 9

[j.j.]

Blink tries it again
Intent to Ship: Array.prototype.values

https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yX4U__z67xo

Comment 10

[Tom S [:evilpie]]

https://groups.google.com/forum/#!topic/mozilla.dev.platform/s92kdFNjL0U

Comment 11

[Tom S [:evilpie]]

Attachment: Add a pref to disable Array.prototype.values

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.

Comment 12

[Jan de Mooij [:jandem]]

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.

Comment 13

[Tom S [:evilpie]]

Attachment: Add default enabled pref for Array.prototype.values

Now that I added the pref to all.js, I also changed it to be default enabled, which matches most of our other preferences.

Comment 14

[Boris Zbarsky [:bzbarsky]]

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.

Comment 15

[Jan de Mooij [:jandem]]

(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?

Comment 16

[Tom S [:evilpie]]

Attachment: v2 - Add default enabled pref for Array.prototype.values

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.

Comment 17

[André Bargull [:anba]]

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

[Jan de Mooij [:jandem]]

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 :)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ca344ca6b5
Add default enabled pref for Array.prototype.values. r=jandem,bz

Comment 20

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e1ca344ca6b5

Comment 21

[Kohei Yoshino]

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/array-prototype-values-is-now-enabled-again/

Comment 22

[Florian Scholz (Open Web Docs)]

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