1009078 - Empty slots at end of array incorrectly represented in console

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Kyle Simpson]

Attachment: ffbug1.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release)
Build ID: 20140508030203

Steps to reproduce:

var a = new Array(4);
a[0] = 0;
a[1] = 1;
a;


Actual results:

In console:

Array [ 0, 1, , ,  ]


Expected results:

In console:

Array [ 0, 1, ,  ]

This representation correctly indicates that there are 4 elements, 2 of which are filled and 2 of which are empty slots. The current representation implies 5 overall slots, the last 3 of which are empty.

Comment 1

[André Bargull [:anba]]

`[ 0, 1, ,  ]` represents an array of length 3, whereas `[ 0, 1, , ,  ]` is an array of length 4. This should explain the current representation.

Comment 2

[Kyle Simpson]

That makes no sense, why would there be an extra trailing slash if it wasn't to indicate an empty slot between the final , and the closing ] character?

Morever, if no empty slot:

var a = new Array(4);
a[0] = 0; a[1] = 1; a[2] = 2; a[3] = 3;
a; // Array [ 0, 1, 2, 3 ]

There you can see there is no trailing slash after the 3, so if a[3] is still an empty slot, it makes no sense that there'd be **2** trailing commas to represent the *single* empty slot.

var a = new Array(4);
a[0] = 0; a[1] = 1; a[2] = 2;
a; // Array [ 0, 1, 2, ,  ]

Comment 3

[André Bargull [:anba]]

Err, I meant to say it represents an array of length 3 resp. 4 when you execute the console output as actual JavaScript source code. So `[ 0, 1, ,  ].length` evaluates to 3, but `[ 0, 1, , ,  ].length` is 4.

Comment 4

[Dave Camp (:dcamp)]

The console is generating a string that when parsed as a literal will be of the correct length.  It's not particularly intuitive, but nor is javascript in this instance.

But if we just include 'undefined' in empty slots[1], we can get behavior that is both correct and intuitive.

[1] chromium's collapsing of consecutive undefined values is nice.

Comment 5

[Kyle Simpson]

> generating a string that when parsed as a literal will be of the correct length

What use-case is that supporting? I'm not previously aware of that being **intentionally** true in any of Chrome's (nor FF) console output. Are there other examples where FF goes out of its way to represent something in a specific way that can be copy-pasted back into the console?

For example:

function foo(){}
var a = new foo();
a; // Object { }  --> chrome says: foo {}

Neither one of those is valid syntax to be dropped back in the console, unless you omit the leading "Object" or "foo" part. Moreover, even a normal object like {a:2, b:3} comes out with the leading "Object" to be omitted.


NOTE: node.js prints out: [ 0, 1, , ] which is, IMO, the best of all 3 outputs thus far.


Anecdotal: I'm pretty familiar with dev-tools and use them in chrome and FF all day. I never once knew this fact, and were I specifically debugging a real program where I printed out an array, and saw the trailing slash there, I would definitely not have though "oh, that's for copy-paste", I would have though "oh, there's a 5th slot there, why the hell is there a 5th slot there, where did that come from, ...."



> if we just include 'undefined' in empty slots

I think that's a bad idea, ESPECIALLY if your goal is to make copying of something from the console back into code and get the same thing. `undefined` is not at all the same behavioral thing as an empty slot, regrettably.

For example:

var a = new Array(4);
a; // let's say you print out: [ undefined, undefined, undefined, undefined ]
a.map(function(_,idx){ return idx; }); // and you also print out: [ undefined, undefined, undefined, undefined ]

Now, someone copies one of those outputs, and puts it back in:

[ undefined, undefined, undefined, undefined ].map(function(_,idx){ return idx; }); // [0, 1, 2, 3]

Oops.

The problem is, an empty slot only behaves like `undefined` ONLY when you retrieve the value, like a[2]. Otherwise, it behaves like it doesn't exist (it doesn't), and therefore isn't iterated on, etc.

I think that web consoles should help represent it in some way that makes it clear that undefined is not the same thing as an empty slot, so they hopefully can avoid this gotcha. Calling it "undefined" does the exact opposite, it goes toward pretending they are the same when they're not.


***
FWIW, I think FF's current empty-commas behavior is very good (like node's), with the only exception of this trailing slash thing (apparently only for copy-paste sake) that node does not do, which IMO makes node's the most accurate and the least likely to mislead developers.
***



> chromium's collapsing of consecutive undefined values is nice.

I actually think chrome's handling of it is much, MUCH worse. It's misleading to imply that "undefined x 2" behaves the same as "undefined undefined", as explained above, AND it would be invalid copy-paste syntax, so it would fail in both respects. :)

Comment 6

[Dave Camp (:dcamp)]

Literal cut-n-paste isn't an explicit use case, but we don't want to confuse someone that knows how to read javascript literals.

Open to other suggestions, but we're not going to strip the trailing comma.

Comment 7

[Kyle Simpson]

You only need the **extra** trailing comma (to preserve the array literals grammar quirk introduced in ES5 to allow trailing commas as non-errors) if you are representing it as a space.

Could you instead say:

[ 0, 1, _, _ ]

Or:

[ 0, 1, {empty}, {empty} ]

Or:

[ 0, 1, █, █ ]   (where those blocks could be styled as grayed-out or whatever)


Literally anything other than a grammatically interpreted whitespace in these "slots" in FF's output could alleviate the need to have that extra trailing slash.

As mentioned before, FF currently outputs [ 0, 1, 2, 3 ]  not  [ 0, 1, 2, 3, ].

Comment 8

[Rob Campbell [:rc] (:robcee)]

evaluating any of those representation won't produce valid javascript. I think we should wontfix this.

Comment 9

[Dave Camp (:dcamp)]

There are plenty of web console outputs that don't cut-n-paste to valid javascript.  I do think there's an opportunity to reduce confusion here.

Comment 10

[jsdev42]

I concur with Kyle on this one. Displaying that last comma is confusing. The behavior is inconsistent with the language. Printing [ 0, 1, , ] makes far more sense to me. 

I even like [ 0, 1, {empty}, {empty} ] a little bit more than the extra comma route.

Comment 11

[Dave Camp (:dcamp)]

maybe [0, 1, <2 empty slots>]?  I'd also be ok with <empty> or {empty} or something like that.

Comment 12

[Kyle Simpson]

I'm totally fine with any of those options, as they all alleviate the need for the extra trailing slash. :)

NOTE: I get questions/problems with this console-output inconsistency stuff (both in chrome and ff) while teaching JS workshops, because the console is usually used as a short-cut to writing short JS programs while learning, so they wonder why they get various outputs. I usually just have to <shrug> and say, "I dunno why it says that, but here's what it means..." Whatever you can do to make things clearer not only helps existing developers but learning-developers, which I think is a very important constituency.

If we can get something that works better than the confusion of "undefined" (or whitespace), I plan to take the precedent set here and ask (beg) chrome, node, and the others to follow suit.

Thanks!

Comment 13

[Sami Jaktholm]

Attachment: webconsole-arrays-with-empty-slots.patch

Here's a patch that collapses consecutive empty slots into a single '<n empty slots>' placeholder.

Comment 14

[Dave Camp (:dcamp)]

Comment on attachment 8443392 [details] [diff] [review]
webconsole-arrays-with-empty-slots.patch

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

::: browser/devtools/webconsole/console-output.js
@@ +2185,3 @@
>      for (let item of items) {
> +      if (item === null) {
> +        emptySlots++;

Does this deal correctly with arrays that have slots that are explicitly set to null or undefined?

There should probably be test cases for that case too.

@@ +2218,5 @@
> +
> +  _renderEmptySlots: function(aNumSlots, aAppendComma=true) {
> +    let slotLabel = l10n.getStr("emptySlotLabel");
> +    let slotText = PluralForm.get(aNumSlots, slotLabel);
> +    this._text("<" + aNumSlots + " " + slotText + ">");

Some languages don't use "<number><space><noun>", the arrangement of the items should probably be part of the pluralform.

https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_and_Plurals seems to imply that the right solution is to put a "#1" into your localized strings and string replace it here.

Thanks for the patch - with fixes to these two things it should be ready to land!

Comment 15

[Sami Jaktholm]

Attachment: webconsole-arrays-with-empty-slots-v2.patch

Thanks for the feedback. Here's a patch that should address your comments.

> Does this deal correctly with arrays that have slots that are explicitly set
> to null or undefined?
Yes it does. Items that are explicitly set to null or undefined are given to the renderer as objects with a type attribute ({type: "null"} or {type: "undefined"}) while really empty slots are represented by null literal. Test case is included in the revised patch.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=993692b01c98

Comment 16

[Dave Camp (:dcamp)]

Comment on attachment 8447591 [details] [diff] [review]
webconsole-arrays-with-empty-slots-v2.patch

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

Looks good.

Apologies for the delay!

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/82020f5ab37f

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/82020f5ab37f