RESOLVED INVALID 82468
Web Inspector: deleting an array property causes it to appear as `undefined × 1` in the console
https://bugs.webkit.org/show_bug.cgi?id=82468
Summary Web Inspector: deleting an array property causes it to appear as `undefined ×...
Mathias Bynens
Reported 2012-03-28 08:13:00 PDT
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?
Attachments
Patch (1.48 KB, patch)
2012-04-25 04:11 PDT, sam
no flags
Patch (112.35 KB, patch)
2012-04-25 08:26 PDT, sam
no flags
Patch (4.55 KB, patch)
2012-04-25 08:44 PDT, sam
no flags
Archive of layout-test-results from ec2-cq-03 (6.17 MB, application/zip)
2012-04-25 10:36 PDT, WebKit Review Bot
no flags
Archive of layout-test-results from ec2-cr-linux-01 (6.26 MB, application/zip)
2012-04-25 14:02 PDT, WebKit Review Bot
no flags
Patch (5.40 KB, patch)
2012-07-11 01:12 PDT, sam
pfeldman: review+
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-01 (349.07 KB, application/zip)
2012-07-11 02:16 PDT, WebKit Review Bot
no flags
Mathias Bynens
Comment 1 2012-03-28 10:27:34 PDT
Another example: `Array(2)` logs `[undefined × 2]`, but `[undefined, undefined]` logs `[undefined, undefined]`.
sam
Comment 2 2012-04-25 04:11:37 PDT
Pavel Feldman
Comment 3 2012-04-25 06:46:43 PDT
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
Comment 4 2012-04-25 07:20:44 PDT
(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
Comment 5 2012-04-25 07:35:50 PDT
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
Comment 6 2012-04-25 08:26:04 PDT
sam
Comment 7 2012-04-25 08:27:55 PDT
(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 8 2012-04-25 08:31:14 PDT
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
Comment 9 2012-04-25 08:44:36 PDT
sam
Comment 10 2012-04-25 09:48:51 PDT
(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 11 2012-04-25 10:36:33 PDT
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
Comment 12 2012-04-25 10:36:40 PDT
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 13 2012-04-25 14:01:08 PDT
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
Comment 14 2012-04-25 14:02:09 PDT
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
Comment 15 2012-07-11 01:12:17 PDT
sam
Comment 16 2012-07-11 01:14:37 PDT
(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 17 2012-07-11 02:16:20 PDT
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
Comment 18 2012-07-11 02:16:29 PDT
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 19 2012-07-11 04:39:08 PDT
Comment on attachment 151639 [details] Patch Please make sure you rebaseline Layouttests/platform/chromium/inspector/console-fromat-expexted.txt prior to landing.
Nick Krempel
Comment 20 2013-02-14 07:42:15 PST
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
Comment 21 2014-12-17 11:21:33 PST
Timothy Hatcher
Comment 22 2015-01-23 13:30:28 PST
Should we distinguish missing and undefined? Close or keep?
Brian Burg
Comment 23 2015-07-19 14:53:20 PDT
Joe's rewrite of array previews has addressed the ambiguity between undefined elements and holes. Closing.
Note You need to log in before you can comment on or make changes to this bug.