Bug 82468 – Web Inspector: deleting an array property causes it to appear as `undefined × 1` in the console

• Status: RESOLVED
• Resolution: INVALID
• Component: Web Inspector

[Mathias Bynens]

Open the console and enter:

    var array = [0, 1, 2, 3]; delete array[0]; array;

This will log the following to the console:

    [undefined × 1, 1, 2, 3]

However, this:

    [undefined, 1, 2, 3];

…will log:

    [undefined, 1, 2, 3]

What’s with the `undefined × 1` thing? Why the different output for these two scenarios? Similarly, the following:

    var array = [0, 1, 2, 3]; delete array[0]; delete array[1]; array;

…logs:

    [undefined × 2, 2, 3]

Which makes more sense, as this saves space compared to logging:

    [undefined, undefined, 2, 3]

Is this documented anywhere?

[Mathias Bynens]

Another example: `Array(2)` logs `[undefined × 2]`, but `[undefined, undefined]` logs `[undefined, undefined]`.

[sam]

Created attachment 138780 [details]
Patch

[Pavel Feldman]

Comment on attachment 138780 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138780&action=review

> Source/WebCore/inspector/front-end/ConsoleMessage.js:341
> +            if (!element || element.textContent === "undefined")

This change does not address the bug. You should instead modify appendUndefined to not render "x 1" in case it is actually "1". The code was not handling the undefined as the first value. This also needs a test (see LayoutTests/inspector/console/console-format.html).

[sam]

(In reply to comment #3)
> (From update of attachment 138780 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138780&action=review
> 
> > Source/WebCore/inspector/front-end/ConsoleMessage.js:341
> > +            if (!element || element.textContent === "undefined")
> 
> This change does not address the bug. You should instead modify appendUndefined to not render "x 1" in case it is actually "1". The code was not handling the undefined as the first value. This also needs a test (see LayoutTests/inspector/console/console-format.html).

Thanks for review Pavel.

I have though of the same thing first. 
But then though of other cases as well while cross checking.

Following are some of the cases and their o/p with checking for render "x 1" in case it is actually "1" in appendUndefined.

1).var array = [0, 1, 2, 3]; delete array[0]; array;
[undefined, 1, 2, 3]

2).var array = [undefined, undefined]; array;
[undefined, undefined]
--- I guess this should be handled as undefined*2. 
It would happen as the undefined values gets missed in while going through array elements and its corresponding properties in the loop just below appendUndefined().
Since there is no delete here, all properties value are in order and there will be no cumulation for undefined.

3).var array = [undefined, undefined, 1, 2, 3]; delete array[2]; delete array[3] ;array;
[undefined, undefined, undefined × 2, 3]
--- Same here.


But I guess check for "1" should be in place as "undefined" is better than "undefined x 1".

Kindly suggest.

[Pavel Feldman]

Here is why delete and simple undefined work differently:

> var a = [undefined, undefined, 3]; Object.keys(a);
["0", "1", "2"]

> var a = [1, 2, 3]; delete a[0]; delete a[1]; Object.keys(a);
["2"]


Anyways, in addition to the fix in appendUndefined, you should then change the loop in the _printArray to ignore values that have type undefined:

for (var i = 0; i < properties.length; ++i) {
    var property = properties[i];
    var name = property.name;
    if (!isNaN(name) >>>>>>>>>>&& property.value.type !== "undefined"<<<<<<<<<<<)
        elements[name] = this._formatAsArrayEntry(property.value);
}

[sam]

Created attachment 138815 [details]
Patch

[sam]

(In reply to comment #5)
> Here is why delete and simple undefined work differently:
> 
> > var a = [undefined, undefined, 3]; Object.keys(a);
> ["0", "1", "2"]
> 
> > var a = [1, 2, 3]; delete a[0]; delete a[1]; Object.keys(a);
> ["2"]
> 
> 
> Anyways, in addition to the fix in appendUndefined, you should then change the loop in the _printArray to ignore values that have type undefined:
> 
> for (var i = 0; i < properties.length; ++i) {
>     var property = properties[i];
>     var name = property.name;
>     if (!isNaN(name) >>>>>>>>>>&& property.value.type !== "undefined"<<<<<<<<<<<)
>         elements[name] = this._formatAsArrayEntry(property.value);
> }

ohh, thanks Pavel!, have uploaded the patch.

[Pavel Feldman]

Comment on attachment 138815 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138815&action=review

One small nit and it is good to go. You don't need to change localizedStrings here. (resetting r?).

> Source/WebCore/inspector/front-end/ConsoleMessage.js:337
> +                span.textContent = WebInspector.UIString("undefined");

You don't need to localize "undefined" - it is a JavaScript term. The reason it was localized previously was "x %d" fragment.

[sam]

Created attachment 138819 [details]
Patch

[sam]

(In reply to comment #8)
> (From update of attachment 138815 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138815&action=review
> 
> One small nit and it is good to go. You don't need to change localizedStrings here. (resetting r?).
> 
> > Source/WebCore/inspector/front-end/ConsoleMessage.js:337
> > +                span.textContent = WebInspector.UIString("undefined");
> 
> You don't need to localize "undefined" - it is a JavaScript term. The reason it was localized previously was "x %d" fragment.

okk, have removed it from localizedStrings. 

Is it ok ?

[WebKit Review Bot]

Comment on attachment 138819 [details]
Patch

Rejecting attachment 138819 [details] from commit-queue.

New failing tests:
inspector/console/console-format.html
Full output: http://queues.webkit.org/results/12520608

[WebKit Review Bot]

Created attachment 138839 [details]
Archive of layout-test-results from ec2-cq-03

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 138819 [details]
Patch

Attachment 138819 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12528690

New failing tests:
inspector/console/console-format.html

[WebKit Review Bot]

Created attachment 138872 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[sam]

Created attachment 151639 [details]
Patch

[sam]

(In reply to comment #15)
> Created an attachment (id=151639) [details]
> Patch

Hi Pavel,

I have uploaded the patch again (content remains the same) as I guess it could not be committed earlier due bot error.

[WebKit Review Bot]

Comment on attachment 151639 [details]
Patch

Attachment 151639 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13180381

New failing tests:
inspector/console/console-format.html

[WebKit Review Bot]

Created attachment 151654 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Pavel Feldman]

Comment on attachment 151639 [details]
Patch

Please make sure you rebaseline Layouttests/platform/chromium/inspector/console-fromat-expexted.txt prior to landing.

[Nick Krempel]

This doesn't seem like a bug to me. I see the '[undefined × 1]' output as a feature, which lets you distinguish between sparse arrays and arrays filled with the value 'undefined'. If you're going to remove the '× 1' part, then perhaps you should also:

(a) Replace 'undefined' with 'missing', so that:

   Array(1)                 =>   [missing]
   Array(2)                 =>   [missing × 2]

(ideally with 'missing' in italics, but not essential.)

(b) Also group genuine 'undefined' values, so that:

   [undefined]              =>   [undefined]
   [undefined, undefined]   =>   [undefined × 2]

I see (a) as more important than (b), and also an easy change.

[Radar WebKit Bug Importer]

<rdar://problem/19281480>

[Timothy Hatcher]

Should we distinguish missing and undefined? Close or keep?

[Brian Burg]

Joe's rewrite of array previews has addressed the ambiguity between undefined elements and holes. Closing.