New issue
Advanced search Search tips

Issue 666882 link

Starred by 1 user
Status: Fixed
Owner:
l...@chromium.org
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 405845



Sign in to add a comment

DevTools: make array formats more consistent (console.log vs normal evaluations/dirxml)

Project Member Reported by l...@chromium.org, Nov 18 2016 
var a = [];
a.length = 10;
a[5] = 1;

console.log(a);
console.dirxml(a);

There's currently 2 array formats with previews, one we use for normal evaluations and dirxml, and another for everything else  (console.log).

It's better to have a single consistent format.

Comment 1 by l...@chromium.org, Nov 18 2016 

Note, the 2nd format (evaluations/dirxml) show inherited array entries and accessor entries while the former does not.

Comment 2 by lushnikov@chromium.org, Nov 19 2016 

Does it relate to any particular step in your design doc?

Comment 4 by l...@chromium.org, Nov 23 2016

Summary: DevTools: make array formats more consistent (console.log vs normal evaluations/dirxml) (was: DevTools: merge array formats (console.log vs normal evaluations/dirxml))
At the very least, if one format shows getters, so should the other.  Even if we don't get rid of one of the formats, we can make them more consistent.
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 2 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/339d1441a7f5c27e48047e825384b734e26790c4

commit 339d1441a7f5c27e48047e825384b734e26790c4
Author: luoe <luoe@chromium.org>
Date: Fri Dec 02 01:41:08 2016

DevTools: separate array formatter from object formatter

No visual or functional changes are presented in this CL. It separates the
_appendArrayPropertiesPreview() to prepare for moving array formatting logic
from printArrayResult(). The logic in printArrayResult() cannot be moved there
yet because it does not make use of the ObjectPreview, but it will do so in a
different CL.

BUG= 666882 

Review-Url: https://codereview.chromium.org/2527773002
Cr-Commit-Position: refs/heads/master@{#435814}

[modify] https://crrev.com/339d1441a7f5c27e48047e825384b734e26790c4/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/644d46fd466d1d893ea6cc6b042a30bce66f3c98

commit 644d46fd466d1d893ea6cc6b042a30bce66f3c98
Author: luoe <luoe@chromium.org>
Date: Wed Dec 07 00:53:03 2016

DevTools: allow array previews to show static array getters

This CL introduces the ability for the latter to properly render getter
properties when they are provided. Objects will continue to exclude show getter
properties in their preview, as is today.

By this change, ConsoleViewMessage's printArrayResult will no longer depend on
property descriptors, bringing us closer to moving all preview logic into
RemoteObjectPreviewFormatter.

Corresponding V8 CL: https://codereview.chromium.org/2508423002/

BUG= 666882 

Review-Url: https://codereview.chromium.org/2521513006
Cr-Commit-Position: refs/heads/master@{#436797}

[modify] https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter-expected.txt
[modify] https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter.html
[modify] https://crrev.com/644d46fd466d1d893ea6cc6b042a30bce66f3c98/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js

Comment 7 by lushnikov@chromium.org, Dec 8 2016 

my 2c: on the screenshot, neither console.log(m), nor just "m" showed the correct array.
console-crazy.png
19.4 KB View Download

Comment 8 by l...@chromium.org, Dec 9 2016

proposedArrayformat.png
16.1 KB View Download
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50c41a1483a808a3394581c717a18cf00773877e

commit 50c41a1483a808a3394581c717a18cf00773877e
Author: luoe <luoe@chromium.org>
Date: Tue Dec 13 21:48:54 2016

DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG= 666882 

Review-Url: https://codereview.chromium.org/2566443004
Cr-Commit-Position: refs/heads/master@{#438294}

[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-array-prototype-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-collections-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-save-to-temp-var-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-tainted-globals-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-inline-values-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-save-to-temp-var-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50c41a1483a808a3394581c717a18cf00773877e

commit 50c41a1483a808a3394581c717a18cf00773877e
Author: luoe <luoe@chromium.org>
Date: Tue Dec 13 21:48:54 2016

DevTools: merge array formatting logic

This CL makes array formatting consistent and prepares
RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
Today, array formatting is split between two files. The logic in
ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
with 'undefined x n' as much as possible then falling back to 'key: value'.

For background on character cutoff logic, see C1 in the console preview doc.

BUG= 666882 

Review-Url: https://codereview.chromium.org/2566443004
Cr-Commit-Position: refs/heads/master@{#438294}

[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-array-prototype-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-collections-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-save-to-temp-var-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/console/console-tainted-globals-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-inline-values-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-save-to-temp-var-expected.txt
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
[modify] https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 17 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/631b841680800d7ea174cc898a4f0c5884111b91

commit 631b841680800d7ea174cc898a4f0c5884111b91
Author: luoe <luoe@chromium.org>
Date: Sat Dec 17 02:17:58 2016

Revert of DevTools: merge array formatting logic (patchset #7 id:120001 of https://codereview.chromium.org/2566443004/ )

Reason for revert:
Console previews for arrays have been compromised.  When logging an array with 100+ non-empty properties, the overflow properties (after the first 100 in the preview) show up as 'undefined x N' when they are not undefined.

Repro case:
var b = new Array(200);
b.fill(2);
console.log(b);

 crbug.com/675259 

Original issue's description:
> DevTools: merge array formatting logic
>
> This CL makes array formatting consistent and prepares
> RemoteObjectPreviewFormatter (ROPF) to introduce character cutoff logic.
> Today, array formatting is split between two files. The logic in
> ConsoleViewMessage has been folded into ROPF.js with all arrays showing gaps
> with 'undefined x n' as much as possible then falling back to 'key: value'.
>
> For background on character cutoff logic, see C1 in the console preview doc.
>
> BUG= 666882 
>
> Committed: https://crrev.com/50c41a1483a808a3394581c717a18cf00773877e
> Cr-Commit-Position: refs/heads/master@{#438294}

TBR=lushnikov@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 675259 

Review-Url: https://codereview.chromium.org/2586623002
Cr-Commit-Position: refs/heads/master@{#439286}

[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-format-array-prototype-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-format-collections-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-save-to-temp-var-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/console/console-tainted-globals-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-inline-values-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-save-to-temp-var-expected.txt
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
[modify] https://crrev.com/631b841680800d7ea174cc898a4f0c5884111b91/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05dd76a462c939c4dcdcce29cf29f10943ace9f9

commit 05dd76a462c939c4dcdcce29cf29f10943ace9f9
Author: luoe <luoe@chromium.org>
Date: Tue Jan 03 21:33:24 2017

Reland of DevTools: merge array formatting logic

This CL makes array formatting consistent. ConsoleViewMessage lets
RemoteObjectPreviewFormatter checks for arrays and handle them, showing gaps
('undefined x n') only when the preview is guaranteed to hold all render-able
properties.

The original version of this CL was reverted:
https://codereview.chromium.org/2566443004/

BUG= 666882 

Review-Url: https://codereview.chromium.org/2596133002
Cr-Commit-Position: refs/heads/master@{#441212}

[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-format-array-prototype-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-format-collections-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-format-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-format.html
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-log-object-with-getter-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-object-preview-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-object-preview.html
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-save-to-temp-var-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/console/console-tainted-globals-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-inline-values-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/debugger-save-to-temp-var-expected.txt
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/Source/devtools/front_end/components/RemoteObjectPreviewFormatter.js
[modify] https://crrev.com/05dd76a462c939c4dcdcce29cf29f10943ace9f9/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js

Comment 13 by l...@chromium.org, Feb 13 2017

Status: Fixed (was: Started)
►

Sign in to add a comment