1608571 - Expanding objects makes them jump around

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect, P3)

Description

[:Harald Kirschner :digitarald]

Attachment: ScreenFlow.gif

Expanding objects and arrays changes their representation, often to something shorter. visual jitter.

Inspection should aim to keep objects in place.

Comment 1

[:Harald Kirschner :digitarald]

Nicolas, is there some prior arguments on why expanded objects change shape?

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

The argument was made in the very beginning of the ObjectInspector, and I can't find the discussion anymore .
I agree that it can be weird sometimes.

This should be a simple fix if we want to do it: remove this block devtools/client/debugger/packages/devtools-reps/src/object-inspector/components/ObjectInspectorItem.js#168-170 and fix the tests that would fail because of this change.

Comment 3

[:Harald Kirschner :digitarald]

This will probably need some hands-on experimentation to get right. Opening up for good-first-bug in case somebody who likes to tinker in UI wants to play around with this.

Different use cases should be considered (single entries, multiple entries in one log, narrow and wide viewport) and the amount of content jumping erratically reduced when expanding arrays/objects/etc.

Comment 4

[Samarjeet]

I would like to work on this

Comment 5

[:Harald Kirschner :digitarald]

Thank you Samarjeet. Nicolas pointed to the right files that should get you started. As this might not be straightforward, feel free to share video (like Loom) recordings of any ideas you want to bounce off.

Comment 6

[Samarjeet]

I can't find an object to reproduce it. Tried many levels of nesting. Could you explain how to reproduce this bug?

Comment 7

[:Harald Kirschner :digitarald]

Attachment: array and object.gif

In this example, clicking the Array pushes the Object around. Not shown, a smaller width expanding the Object pushes it down one line.

Other STR:

• Log console.log(window, document)
• Resize the console so both elements fit on the same line
• Expand one

AR: Clicked Objects jump around.
ER: Layout stays more stable

Comment 8

[Samarjeet]

Attachment: jumping.gif

Is this the undesired behaviour?

Comment 9

[Samarjeet]

Attachment: case2.gif

Or is this?

Comment 10

[:Harald Kirschner :digitarald]

I thought that it would be easiest if the Array(3) [ 1, 2, 3 ] would stay in place while additional content expands below.

The forced line break from comment 9 also makes sense, ensuring expanded objects get the space they need at all times.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Samarjeet, are you still interested in fixing this bug?

Honza

Comment 12

[Samarjeet]

Oh yes, I am. This had slipped earlier.

Comment 13

[Samarjeet]

Hi, actually I am working on another bug at the moment. I don't think it's ethical for me to be working on two bugs at a time, it'll just delay it.
Let's open this up for contributors. I'll try getting back to this once the other bug is done.

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Thanks for the heads-up Samarjeet, I unassigned the bug.

Comment 15

[Mary S. :csfj87]

Hi, can I be assigned to this bug?

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Hello Mary, the bug is now yours

If it's your first bug, you might want to read this https://firefox-source-docs.mozilla.org/devtools/getting-started/README.html to setup the work environment.
Make sure to select "Artifact builds" when asked as it speeds up the workflow quite a bit :)
Also, you can come and chat on our Slack https://devtools-html-slack.herokuapp.com/

When doing changes for that patch, you'll need to re-generate the bundle we have each time you're testing a change:

cd devtools/client/debugger
node bin/bundle

don't hesitate to ask any question here or on Slack

Comment 17

[Mary S. :csfj87]

Thanks for the info Nick!

Comment 18

[Mary S. :csfj87]

Attachment: Terminal view of error I ran into

Hey Nick,

I ran into this error when I used the re-generate bundle code. I don't know where to find devtools-launchpad/index.

Comment 19

[Nicolas Chevobbe [:nchevobbe]]

Hello Mary,

Could you try to do yarn install before? We need to pull those dependencies from npm :)

Comment 20

[Mary S. :csfj87]

Hey Nick,

yarn install worked! I pushed a commit and added you as the reviewer. My only question now is, is there a revision ID for this bug?

Comment 21

[Nicolas Chevobbe [:nchevobbe]]

Hello Mary,

I don't see the patch anywhere? What did you use to push?
Is the commit message like Bug 1608571 - Don't shorten object when expanding. r=nchevobbe. ?

Comment 22

[Mary S. :csfj87]

Attachment: TerminalImage2.png

I used hg commit and the message is Bug 1608571 - Fixes expanding Object issue. r=nchevobbe. I realized I forgot to use moz-phab submit, but now when I try to use it I get an API error.

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

can you try to run moz-phab install-certificate again?

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 25

[Karthik Sundar]

Can I take this bug??, If it is still open

Comment 26

[Karthik Sundar]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)

The argument was made in the very beginning of the ObjectInspector, and I can't find the discussion anymore .
I agree that it can be weird sometimes.

This should be a simple fix if we want to do it: remove this block devtools/client/debugger/packages/devtools-reps/src/object-inspector/components/ObjectInspectorItem.js#168-170 and fix the tests that would fail because of this change.

I tried to look into the file mentioned. However, I was not able to locate the folder devtools-reps.
I have only the following folders in the packages directory.

1. devtools-source-map
2. devtools-splitter
3. devtools-utils
4. devtools-wasm-dwarf

Comment 27

[Nicolas Chevobbe [:nchevobbe]]

Hello Karthik, sure you can give it a try.
The file was moved to https://searchfox.org/mozilla-central/rev/63fcc3f1a2cc73488d8986f4cf91fce2cd4b7564/devtools/client/shared/components/object-inspector/components/ObjectInspectorItem.js#123-125

Comment 28

[Karthik Sundar]

In Comment 8 and 9, two cases are given, which is the undesired behaviour? Also, it will be really helpful, if I get some testcases to test my code, for instance the code in the Screenflow.gif.

Comment 29

[Nicolas Chevobbe [:nchevobbe]]

None of them seem to picture what's wanted.
Let me recap:

1. Open the console
2. Evaluate [1,2,3] (
3. We now have ▶︎ Array(3) [ 1, 2, 3 ] printed in the console
4. Click on the arrow to expand the object
5. We now have

▼ (3) […]
|   0: 1
|   1: 2
|   2: 3
|   length: 3
| ▶︎ <prototype>: Array []

the purpose of this bug is to have

▼ (3) Array(3) [ 1, 2, 3 ]
|   0: 1
|   1: 2
|   2: 3
|   length: 3
| ▶︎ <prototype>: Array []

instead (so the first line stays the same, we only expand the arrow)
It should limit some of the jumping we see.

Comment 30

[Karthik Sundar]

Attachment: Result.png

I have been able to achieve this result. Shall I submit my code for review???

Comment 31

[Nicolas Chevobbe [:nchevobbe]]

sure! You can follow https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-write-a-patch and https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-submit-a-patch to do that

Comment 32

[Karthik Sundar]

Attachment: Bug 1608571- Removed the code responsible for shortening to remove the jumping around of objects when expanding. r=nchevobbe.

Comment 33

[Karthik Sundar]

I just want to notify that I have submitted my patch. I hope I didn't do anything wrong in submitting my patch. If any issues kindly contact me.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22ea220f8502
Removed the code responsible for shortening to remove the jumping around of objects when expanding. r=nchevobbe.

Comment 35

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/22ea220f8502