Bug 29714 – apply a style change to scope variables as they change from step to step

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

[Patrick Mueller]

As the user is stepping through their code in Web Inspector, it would be handy to be able to highlight values which have changed from before they made a step to the next pause state.

Lots of thoughts immediately come to mind:

- how do you tell when the user has just 'stepped" vs hitting a breakpoint after a run?  Presumably it only makes sense to show items that have changed when you're stepping, rather than since the last pause state to the current pause state.

- how deeply do we check for changes?  Just the top level properties in the various sections?  The leafs in the expanded sections?

- presumably new properties/variables would get marked as "changes" - as well as a normal property change; what about property/variable deletions?  I'm imagining showing some ghostly or strike-out representation of the deleted property/variable

- how do we visually indicate "changed".  Note this also needs to take into account property value styling as per bug 27235 .  Namely, we can't use the same style for both.  I'm thinking a background color - maybe yellow-ish - could be used to indicate "changed".  Rounded corners on the background of course!  This might preclude using background colors for value styling, which we might want to do for 'special' values such as exceptions, null, undefined, NaN, Infinity, etc.  Though if those values used a dark background, the combination of backgrounds might be distinctive enough to be able to easily tell that both characteristics are applicable.  Alternatively, the "changed" styling background might extend to the left of the usual margin for the property value, so you could visually distinguish that as well.

[Sandip Chitale]

I would like this feature also.

I was trying to implement a patch for it, but ran into an issue of stability of the runtime.RemoteObject.objectId. I appears that this id is not the same for the same object (say window object) between successive stopping of the debugee. This prevents the frontend based implementation of diff computation. May be the backend should simply set and additional property of runtime.RemoteObject, say state to one of added or changed which can be used by the frontend...more precisely near this code in DevTools.js:

:
WebInspector.ObjectPropertyTreeElement.prototype = {
:
:
:
update: function()
{
:
:
this.nameElement.textContent = this.property.name;
if ('added' == this.property.value.state) {
    // add a class to the to row to indicate new property
} else if ('changed' == this.property.value.changed) {
    // add a class to the to row to indicate change property
}
:
:

[Sandip Chitale]

Created attachment 189962 [details]
Possible patch to implement the functionality

This is my first attempt at implementing the functionality of applying a style change to scope variables as they change from step to step. The idea is simple - keep track of values that the user saw in last iteration for any expanded nodes. Next time compare, if different apply a different style class.

[Sandip Chitale]

Just added a possible patch. I still need to refine it further.

[Sandip Chitale]

Created attachment 190223 [details]
Improved patch. Now computes diffs relative to the nodes that were shown when the debugger was suspended last.

[Sandip Chitale]

Created attachment 190224 [details]
step 1 screenshot

[Sandip Chitale]

Created attachment 190225 [details]
step 2 screenshot

[Sandip Chitale]

Created attachment 190226 [details]
step 3 screenshot

[Sandip Chitale]

Created attachment 190228 [details]
step 4 screenshot

[Sandip Chitale]

Created attachment 190229 [details]
step 5 screenshot

[Pavel Feldman]

> - how deeply do we check for changes?  Just the top level properties in the various sections?  The leafs in the expanded sections?

We should compare primitive values and rely on object identity.

> 
> - presumably new properties/variables would get marked as "changes" - as well as a normal property change; what about property/variable deletions?  I'm imagining showing some ghostly or strike-out representation of the deleted property/variable

We should not go deep. New properties in the scope are just new scope variables.

[Vivek Galatage]

Comment on attachment 190223 [details]
Improved patch. Now computes diffs relative to the nodes that were shown when the debugger was suspended last.

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

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:106
> +        if (this.pane) {

We usually omit the braces for single line statements in an if block. Here and below all the occurrences should remove the braces for a single line statements.

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:243
> +        {

The opening brace is placed after if() {

[Sandip Chitale]

Created attachment 190377 [details]
This patch incorporates the formatting feedback.

[Sandip Chitale]

Aside from incorporating the formatting feedback this patch adds the following:

- Detect when the type of the property changes
- When a new property is detected also highlights the name. For modified properties only highlights the value.

[Sandip Chitale]

Created attachment 190471 [details]
The previous patch broke the Watch Expressions pane. This fixes it.

Had to make changes to WatchExpressionsSidebarPane.js to fix the Watch Expressions window.

[Sandip Chitale]

How was this fixed? Did the patch work? When will be available?

[Brian Burg]

(In reply to comment #15)
> How was this fixed? Did the patch work? When will be available?

Hi Sandip, we have been closing out old bugs filed against the "Web Inspector (Deprecated)" component. I closed this because there's been no activity in over a year. However it is not "fixed".

If you intend to keep working on it, I'm happy to migrate it to the correct component and reopen it. You will probably need to start over with a new patch, as the posted patches are based on the old inspector UI.

[Sandip Chitale]

Hi Brian,

Please migrate the bug. I will submit the patch again.

Thanks,
Sandip

[Brian Burg]

Reopening..

[Sandip Chitale]

Created attachment 243123 [details]
This is a patch against the latest code

This is a patch against the latest code at: https://chromium.googlesource.com/chromium/blink

[Brian Burg]

(In reply to comment #19)
> Created attachment 243123 [details]
> This is a patch against the latest code
> 
> This is a patch against the latest code at:
> https://chromium.googlesource.com/chromium/blink

Blink is a separate codebase with a different inspector UI implementation. Thus, this patch is not reviewable. Please see the above comments.

[Sandip Chitale]

Hi Brian,

What is the code base against which I should create the patch. I mean the git or SVN repository?

Thanks,
Sandip

[Joseph Pecoraro]

(In reply to comment #21)
> What is the code base against which I should create the patch. I mean the
> git or SVN repository?

Report bugs and attach patches to the Chromium bug tracker:
https://code.google.com/p/chromium/issues/entry

[Sandip Chitale]

Done.

https://code.google.com/p/chromium/issues/detail?id=441374