DevTools: show alternate title onexpand of object in console

DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682
Committed: https://crrev.com/e3bc1d8cebf65a51bb23abf204cf1ba059f1e865
Cr-Commit-Position: refs/heads/master@{#410100}

[luoe, 2016-07-11 23:04:54]

Description was changed from

==========
DevTools: show alternate title onexpand of object in console

BUG=607682
==========

to

==========
DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682
==========

[luoe, 2016-07-11 23:08:06]

luoe@chromium.org changed reviewers:
+ lushnikov@chromium.org

[luoe, 2016-07-11 23:08:06]

This CL should actually pass LayoutTests!  Please take a look.

[lushnikov, 2016-07-12 02:51:55]

Let's maybe try Node.replace() instead hiding elements - this will keep tests
clean + we'll safe a nit on layout :)

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:32:
* @param {?string|!Element=} expandedTitle
let's only have Element here

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:55:
if (typeof expandedTitle === "string") {
this will go away once you have only Element as input

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:64:
this.expandedTitleElement.classList.add("hidden");
Let's use hidden attribute here and everywhere

this.expandedTitleElement.hidden = true;

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:332:
this.valueElement.classList.add("hidden");
let's extract a _showExpandedValueElement(visible) and call it in onexpand,
oncollapse and _editingCanceled

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:376:
if (this.property.value && valueText && valueText !==
this.property.value.description && !this.property.wasThrown)
lets drop this optimization as well

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:436:
if (this.expandedValueElement)
i believe we'll always have expandedValueElement once we drop that
optimization...

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1296:
var type = value.type;
let's inline type, subtype & desct

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:443:
expandedTitleElement.createTextChild(obj.description || "");
let's just create a TextNode with obj.description || "" and pass it down to
ObjectPropertiesSection

[luoe, 2016-07-13 18:06:28]

Thanks for the comments!  Patches 2+3 need review, here is a summary of changes:

- No more hiding.  Instead, toggling the expand arrow swaps the childNodes
between the titleElement and expandedTitleElement.
- The info note (says 'this object was snapshotted...' will be shown on the
expandedTitleElement).  Previously, it was hidden via css.
- Tests were updated
-- console-format-broken-unicode.html now requires 2 expands in order to see the
expected result, so I added in another expandConsoleMessages with a custom
filter
-- console-proxy doesn't actually need to dump the display of the info-note,
since the note is now always shown whenever expandedTitleElement is visible
-- Instead of adding a new test just for collapsed previews, I thought it would
be better to have certain tests dump messages before and after expand
(console-format, console-format-es6, console-format-es6-2,
console-object-preview).  By doing this, anyone who breaks the way an es6 object
is formatted will just have to look through one failed test.

Ptal.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:32:
* @param {?string|!Element=} expandedTitle
On 2016/07/12 02:51:55, lushnikov wrote:
> let's only have Element here

Done.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:55:
if (typeof expandedTitle === "string") {
On 2016/07/12 02:51:55, lushnikov wrote:
> this will go away once you have only Element as input

Done.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:64:
this.expandedTitleElement.classList.add("hidden");
On 2016/07/12 02:51:55, lushnikov wrote:
> Let's use hidden attribute here and everywhere
> 
> this.expandedTitleElement.hidden = true;

Done.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:332:
this.valueElement.classList.add("hidden");
On 2016/07/12 02:51:55, lushnikov wrote:
> let's extract a _showExpandedValueElement(visible) and call it in onexpand,
> oncollapse and _editingCanceled

No longer needed when children are swapped.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:376:
if (this.property.value && valueText && valueText !==
this.property.value.description && !this.property.wasThrown)
On 2016/07/12 02:51:55, lushnikov wrote:
> lets drop this optimization as well

Done.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:436:
if (this.expandedValueElement)
On 2016/07/12 02:51:55, lushnikov wrote:
> i believe we'll always have expandedValueElement once we drop that
> optimization...

We still need this because there are a few exceptions when an
expandedValueElement is not created (when a customPreview is provided by a
custom formatter, DOM Nodes, functions)

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1296:
var type = value.type;
On 2016/07/12 02:51:55, lushnikov wrote:
> let's inline type, subtype & desct

Done.

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2139043002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:443:
expandedTitleElement.createTextChild(obj.description || "");
On 2016/07/12 02:51:55, lushnikov wrote:
> let's just create a TextNode with obj.description || "" and pass it down to
> ObjectPropertiesSection

One reason to keep expandedTitleElement an HTMLElement is that we can add the
snapshot-info-note on #435 as a child of the expandedTitleElement.  Then on
expand/collapse, the expandedTitleElement's children can be swapped in and out.

[luoe, 2016-07-14 21:51:34]

Summary of patches 4+5+6+7:

- When editing a property via double click, a property's valueElement needs the
'hidden' class, not the hidden attribute, which gets overridden by its 'display'
css value.  I've added a test that validates behavior when double-clicking to
edit a property value (console-edit-property-value.html)
- Double click to edit used to call applyExpression() which called
parent.expand().  Now it just calls parent.onpopulate(), since the parent's
expanded state shouldn't change after a child was edited.
- TreeElement.insertChild(child) no longer tries to child.expand(). 
TreeElement.expand() should exit if the element.expanded == true.  Due to this
condition, I removed the _children check from treeoutline.js:888.  However, this
broke behavior when expanding an object with [[Entries]] (e.g. Maps, sets),
where expanding the map should automatically expand its entries property.  Thus,
I changed the order of logic in ObjectPropertiesSection.js:627 so that when
adding the "[[Entries]]" internal property, we add the property to the tree
BEFORE calling expand on it.

Ptal.

[luoe, 2016-07-14 23:45:09]

Removed unrelated changes.

[lushnikov, 2016-07-20 23:54:26]

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:38:
WebInspector.ObjectPropertiesSection = function(object, title, expandedTitle,
linkifier, emptyPlaceholder, ignoreHasOwnProperty, extraProperties)
Let's look if we can reduce amount of arguments here before adding the new, 8th,
argument

There's a good chance that ObjectPropertiesSection could be simplified at first
before complicating it
even further

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:436:
if (obj.description &&
WebInspector.ObjectPropertiesSection.needsAlternateTitle(obj)) {
> ... && WebInspector.ObjectPropertiesSection.needsAlternateTitle(obj)) {

can we hide this inside the ObjectPropertiesSection?

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/platform/DOMExtension.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/platform/DOMExtension.js:738:
Node.prototype.swapChildren = function(node) {
let's inline this for now

[luoe, 2016-07-22 22:30:58]

Comments addressed.

1 additional change:
needsAlternateTitle() has been modified so that objects with type === "object"
but without a preview do not get an alternate title.  This is done so that
normal objects with a 'special title' keep their special title on expand (for
example, LayoutTests/inspector/local-object-properties-section.html)

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:38:
WebInspector.ObjectPropertiesSection = function(object, title, expandedTitle,
linkifier, emptyPlaceholder, ignoreHasOwnProperty, extraProperties)
On 2016/07/20 23:54:26, lushnikov wrote:
> Let's look if we can reduce amount of arguments here before adding the new,
8th,
> argument
> 
> There's a good chance that ObjectPropertiesSection could be simplified at
first
> before complicating it
> even further

Good call, by moving needsAlternateTitle() into OPS, we can avoid adding in a
this argument.

The others seem to have a valid reason to be there, except maybe linkifier.  We
could have OPS expose a setLinkifier(linkifier) function, but that should be a
separate CL.

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js:436:
if (obj.description &&
WebInspector.ObjectPropertiesSection.needsAlternateTitle(obj)) {
On 2016/07/20 23:54:26, lushnikov wrote:
> > ... && WebInspector.ObjectPropertiesSection.needsAlternateTitle(obj)) {
> 
> can we hide this inside the ObjectPropertiesSection?

Done.

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/platform/DOMExtension.js
(right):

https://codereview.chromium.org/2139043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/platform/DOMExtension.js:738:
Node.prototype.swapChildren = function(node) {
On 2016/07/20 23:54:26, lushnikov wrote:
> let's inline this for now

Done.

[luoe, 2016-07-26 19:09:12]

Gentle ping :)

[lushnikov, 2016-07-30 00:29:50]

Thanks, I like this much more!

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-format-broken-unicode.html
(right):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-format-broken-unicode.html:49:
function nestedObjectFilter(treeElement)
can you please explain this filter?

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/inspector/console/console-proxy.html (left):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-proxy.html:36:
dumpNoteVisible(element, "info-note");
do we ever check for info-note now?

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:215:
var currentTitleElement = this.treeOutline.titleElement;
AFAIU this code is there for the sake of editing the value of exandable
property.

And AFAIU, this scenario is not something we have today.
So, can we just disallow editing values of expandable properties?

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:357:
this.valueElement.removeChildren();
same idea as for _toggleExpandedTitleElement might apply here

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1165:
WebInspector.ObjectPropertiesSection.needsAlternateTitle = function(object)
let's make this a private method of WI.ObjectPropertiesSection

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1174:
WebInspector.ObjectPropertiesSection.createExpandedValueElement =
function(value)
let's make this a private method of WI.ObjectPropertiesSection

[luoe, 2016-07-30 01:46:38]

Ptal.

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-format-broken-unicode.html
(right):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-format-broken-unicode.html:49:
function nestedObjectFilter(treeElement)
On 2016/07/30 00:29:49, lushnikov wrote:
> can you please explain this filter?

In the broken unicode test, it evaluates [{ ..interesting stuff... }]
expandConsoleMessages only expands one level, so what we get is this output:
V Array[1]
    > 0: Object

The test wants to make sure that all the interesting stuff is printed correctly,
so another expand is needed.  While we don't want to expand every key another
level, the filter is used to tell it which properties should be expanded.

In the next patch, I can hopefully make it clearer that it's just going to
expand any property "0", which should match the array item and then show:

V Array[1]
    V 0: Object
        ...interesting stuff...

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/inspector/console/console-proxy.html (left):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-proxy.html:36:
dumpNoteVisible(element, "info-note");
On 2016/07/30 00:29:49, lushnikov wrote:
> do we ever check for info-note now?

Yes, 2 other places.

- Further down in this file, there it checks that the note is shown when the
Proxy is expanded

- In console-object-preview.html, we expand and dump out the names of all the
CSS classes, which we expect to include info-note

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:215:
var currentTitleElement = this.treeOutline.titleElement;
On 2016/07/30 00:29:49, lushnikov wrote:
> AFAIU this code is there for the sake of editing the value of exandable
> property.
> 
> And AFAIU, this scenario is not something we have today.
> So, can we just disallow editing values of expandable properties?

True, we can't edit titles via double click.  I'll switch this code to something
nicer.

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:357:
this.valueElement.removeChildren();
On 2016/07/30 00:29:49, lushnikov wrote:
> same idea as for _toggleExpandedTitleElement might apply here

For values, we do currently support editing them via double click, so this
children swapping supports the current editing behavior.

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1165:
WebInspector.ObjectPropertiesSection.needsAlternateTitle = function(object)
On 2016/07/30 00:29:49, lushnikov wrote:
> let's make this a private method of WI.ObjectPropertiesSection

Done.

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:1174:
WebInspector.ObjectPropertiesSection.createExpandedValueElement =
function(value)
On 2016/07/30 00:29:49, lushnikov wrote:
> let's make this a private method of WI.ObjectPropertiesSection

Done.

[lushnikov, 2016-08-02 17:08:07]

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:357:
this.valueElement.removeChildren();
On 2016/07/30 01:46:37, luoe wrote:
> On 2016/07/30 00:29:49, lushnikov wrote:
> > same idea as for _toggleExpandedTitleElement might apply here
> 
> For values, we do currently support editing them via double click, so this
> children swapping supports the current editing behavior.

Yes, but maybe we don't want this scenario at all? It doesn't work
in many cases for me ATM (try edit object into object, or putting "Array[4]"
instead of "Array[5]" and so on)

[luoe, 2016-08-02 22:49:35]

Switched back to simpler replaceChild() method of toggling.

Also removed complex values from the editing test.

[lushnikov, 2016-08-03 22:34:49]

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value-expected.txt
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value-expected.txt:5:
console-edit-property-value.html:10 Objecta: 3b: "foo"c: Array[3]__proto__:
Object
can we use some InspectorTest.dumpObjectPropertyTreeElement() to make output
readable?

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value.html
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value.html:61:
InspectorTest.assertTrue(node.classList.contains("hidden"));
let's not assert in tests - instead, dump and error and complete test.
This will help to debug test with flakiness dashboard if something goes wrong.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:54:
if (object.description &&
WebInspector.ObjectPropertiesSection._needsAlternateTitle(object)) {
Let's not make it static without a need - it's easier to read code from top to
bottom

this._needsAlternateTitle()

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:188:

stray line

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:211:
_toggleExpandedTitleElement: function()
let's make it straightforward: 

/**
 * @param {boolean} value
 */
_setExpandedTitleElement: function(value)

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:349:
_toggleExpandedValueElement: function()
Like with title:

_setExpandedValueElement: function(value)

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:388:
this.expandedValueElement =
WebInspector.ObjectPropertiesSection._createExpandedValueElement(this.property.value);
this._createExpandedValueElement(this.property.value)

[luoe, 2016-08-05 01:12:47]

Thanks for the comments.  Ptal

Notes:
- instead of "setExpandedValueElement", maybe "showExpandedValueElement" is a
better name?
- I don't see an easy way to make _needsAlternateTitle() not static

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value-expected.txt
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value-expected.txt:5:
console-edit-property-value.html:10 Objecta: 3b: "foo"c: Array[3]__proto__:
Object
On 2016/08/03 22:34:48, lushnikov wrote:
> can we use some InspectorTest.dumpObjectPropertyTreeElement() to make output
> readable?

We can make a custom formatter for dumping object properties.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value.html
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/console/console-edit-property-value.html:61:
InspectorTest.assertTrue(node.classList.contains("hidden"));
On 2016/08/03 22:34:48, lushnikov wrote:
> let's not assert in tests - instead, dump and error and complete test.
> This will help to debug test with flakiness dashboard if something goes wrong.

Done.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:54:
if (object.description &&
WebInspector.ObjectPropertiesSection._needsAlternateTitle(object)) {
On 2016/08/03 22:34:49, lushnikov wrote:
> Let's not make it static without a need - it's easier to read code from top to
> bottom
> 
> this._needsAlternateTitle()

Unfortunately, I don't think we can easily make this non-static.  It's used by
OPSection and OPTreeElement and there wasn't a nice way to move it.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:188:

On 2016/08/03 22:34:48, lushnikov wrote:
> stray line

Done.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:211:
_toggleExpandedTitleElement: function()
On 2016/08/03 22:34:48, lushnikov wrote:
> let's make it straightforward: 
> 
> /**
>  * @param {boolean} value
>  */
> _setExpandedTitleElement: function(value)

Done.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:349:
_toggleExpandedValueElement: function()
On 2016/08/03 22:34:49, lushnikov wrote:
> Like with title:
> 
> _setExpandedValueElement: function(value)

Done.

https://codereview.chromium.org/2139043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js:388:
this.expandedValueElement =
WebInspector.ObjectPropertiesSection._createExpandedValueElement(this.property.value);
On 2016/08/03 22:34:48, lushnikov wrote:
> this._createExpandedValueElement(this.property.value)

Done.

[lushnikov, 2016-08-05 01:42:04]

https://codereview.chromium.org/2139043002/diff/340001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/console-test.js
(right):

https://codereview.chromium.org/2139043002/diff/340001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/console-test.js:79:
InspectorTest.dumpConsoleMessagesWithObjectProperties = function(messageElement,
consoleMessage)
let's reuse the existing infrastructure (see console-format.html test)

[luoe, 2016-08-05 01:48:10]

dump() reused

https://codereview.chromium.org/2139043002/diff/340001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/console-test.js
(right):

https://codereview.chromium.org/2139043002/diff/340001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/console-test.js:79:
InspectorTest.dumpConsoleMessagesWithObjectProperties = function(messageElement,
consoleMessage)
On 2016/08/05 01:42:04, lushnikov wrote:
> let's reuse the existing infrastructure (see console-format.html test)

Done.

[luoe, 2016-08-05 01:49:55]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 01:50:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lushnikov, 2016-08-05 01:54:37]

lgtm

[commit-bot: I haz the power, 2016-08-05 03:01:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-05 03:01:49]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-08-05 17:38:12]

The CQ bit was checked by luoe@chromium.org

[commit-bot: I haz the power, 2016-08-05 17:38:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-05 17:43:44]

Description was changed from

==========
DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682
==========

to

==========
DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682
==========

[commit-bot: I haz the power, 2016-08-05 17:43:46]

Committed patchset #19 (id:360001)

[commit-bot: I haz the power, 2016-08-05 17:45:53]

Description was changed from

==========
DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682
==========

to

==========
DevTools: show alternate title onexpand of object in console

Results logged in the console with an expandable arrow will show a full preview
title when collapsed and a description when expanded. For example, an array
"[1, 2, 3]" might expand into "Array[3]".

BUG=607682

Committed: https://crrev.com/e3bc1d8cebf65a51bb23abf204cf1ba059f1e865
Cr-Commit-Position: refs/heads/master@{#410100}
==========

[commit-bot: I haz the power, 2016-08-05 17:45:55]

Patchset 19 (id:??) landed as
https://crrev.com/e3bc1d8cebf65a51bb23abf204cf1ba059f1e865
Cr-Commit-Position: refs/heads/master@{#410100}