1599978 - Sidebar splitter stops working when dragging

Status: VERIFIED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Karl Dubost💡 :karlcow]

Attachment: debugger-tool-long-name

1. Open https://maps.google.com/
2. Open the debugger

Actual:
The left column with the name of resources is maximized to the point where the script central column is not usable. Not possible to manually resize

Expected:

1. Columns being adjusted at a reasonable size by default.
2. It can be resized manually.

This is high priority it's a major hurdle for using the tool. It has appeared recently maybe a couple of weeks ago. I was waiting to see if it would be fixed, but it's not yet.

Firefox Nightly 72.0a1 (2019-11-26) (64-bit)

Comment 1

[Logan Smyth [:loganfsmyth]]

I know we've changed some things around the sidebar, so possibly related to:

• https://bugzilla.mozilla.org/show_bug.cgi?id=1595413
• https://bugzilla.mozilla.org/show_bug.cgi?id=1589272

Comment 2

[Karl Dubost💡 :karlcow]

Not related, but the flex: 1 here has no effect and can be removed.

.split-box {
	display: flex;
	flex: 1;
	min-width: 0;
	height: 100%;
	width: 100%;
}

For the issue at stake
html.theme-light body div#mount div.launchpad-root.theme-body div.debugger div.A11y-mouse div.split-box.vert div.uncontrolled div.split-box.vert div.controlled

This is the max-width here which seems to set a size on the element and forbids it to be smaller.

element {
	max-width: 85%;
	min-width: 30px;
}

resource://devtools/client/debugger/src/components/App.js

        startPanel: _react.default.createElement(_devtoolsSplitter.default, {
          style: {
            width: "100vw"
          },
          initialSize: _prefs.prefs.startPanelSize,
          minSize: 30,
          maxSize: "85%",
          splitterSize: 1,
          onResizeEnd: num => {
            _prefs.prefs.startPanelSize = num;
          },
          startPanelCollapsed: startPanelCollapsed,
          startPanel: _react.default.createElement(_PrimaryPanes.default, {
            horizontal: horizontal
          }),
          endPanel: this.renderEditorPane()
        }),

1. 85% doesn't seem to be reasonable as a start value.
2. The value is not adjusted when dragging the vertical separator. (onResize)

Comment 3

[Karl Dubost💡 :karlcow]

It's like the maxSize is interpreted as a minSize.

Comment 4

[Karl Dubost💡 :karlcow]

it doesn't fix things, but it improves a bit

/* Element | chrome://devtools/content/debugger/index.html */

div.controlled:nth-child(1) {
  /* max-width: 85%; */
  /* min-width: 30px; */
  flex: 1 1 30px;
}

/* vendors.css | resource://devtools/client/debugger/dist/vendors.css */

.split-box > .uncontrolled {
  /* flex: 1; */
  /* min-width: 0; */
  flex: 1 1 30px;
}

Comment 5

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

Attachment: image.png

I can't reproduce the problem on my machine (Win10, Firefox Nightly)
Size of the left side bar (with Sources) can be manually changed in my case.

Karl, is there anything specific I should do?

Honza

Comment 6

[Karl Dubost💡 :karlcow]

Oh that's weird!
Honza, here a video of what I see.
macOS 10.15.1 (19B88) Firefox Nightly 72.0a1 (2019-11-28) (64-bit)
https://imgur.com/a/fM1woB2

Comment 7

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

I tried both Win and Mac and I still can't see the issue.

Logan, can you repro this?

Honza

Comment 8

[Karl Dubost💡 :karlcow]

Let me try to create a completely new profile.
…
…
ok this doesn't reproduce for me in a completely new profile. What could create this issue?
I have a couple of addons related to devtools.
Let's remove them one by one.
…
…
It doesn't reproduce.
Would it be a specific config?
…
…

• devtools.aboutdebugging.collapsibilities.processes: true which doesn't exist in the new profile. I removed it.
• devtools.chrome.enabled: true set it to false again
• devtools.command-button-screenshot.enabled: true to false
• devtools.debugger.dom-mutation-breakpoints-visible: true to false
• devtools.debugger.end-panel-size: 297 back to 300.
• devtools.debugger.start-panel-size: -8770 back to 300

ah this one seems related, after setting back to 300. and trying to grab the vertical sep to move it goes back to a very large negative value.

wow… and I repaired it. Changing the value to 300 didn't solve anything but dragging and drop toward the right side until the value becomes positive again made it working again.

Restarting and this is working too.
So I have no idea… how it became this, but probably there should be a way to detect that the value is not making sense.

Hope it helps. Feel free to close if you think it's a too peculiar issue.

Comment 9

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

Thanks for the analysis Karl, that's super helpful!

I think there is a bug related to how the pref value is stored. I was able to quickly break the separator by moving the mouse far from it when dragging.

Here is my STR:

1. Load about:config and search for devtools.debugger.start-panel-size (so, we can see how value of the pref is changing)
2. Open DevTools and select the Debugger panel
3. Chang size of the left side bar (Sources, Outline). While dragging the vertical splitter move mouse to far right (and perhaps across multitple monitors if you have) - do not stop even if the size of the bar isn't changing anymore.
4. Release the mouse and check the value (I was able to get > 1000)
5. Repeat the same multiple times and also with the right side bar.
6. Observe the behavior - it breaks quickly -> BUG

It looks like we shouldn't store the position of the mouse, but rather actual size of the bar.

@Jason, perhaps we could provide some instructions how to fix it and make it good first bug?

Honza

Comment 10

[Alex J Garcia]

For the start-panel, clientWidth + movementX can be stored as state.width. This ensures that the panel does not grow more than its allowed limit. end-panel is not as straightforward, but I would like to continue working towards a solution. May I be assigned this bug?

Comment 11

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

You are now assigned, thanks for the help!
Honza

Comment 12

[Alex J Garcia]

I have made my changes in vendors.js, you can find my revision here: https://phabricator.services.mozilla.com/D69708

Comment 13

[Alex J Garcia]

Attachment: Bug 1599978 - Store clientX as state.width for start-panel and the difference between clientWidth and clientX for end-panel.

Bug 1599978 - Used correct variable.

Comment 14

[Alex J Garcia]

Attachment: Bug 1599978 - Stores clientX as state.width for start-panel and the difference between clientWidth and clientX for end-panel. r=bomsy

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6d1fb1401f0
Stores clientX as state.width for start-panel and the difference between clientWidth and clientX for end-panel. r=bomsy,davidwalsh

Comment 16

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e6d1fb1401f0

Comment 17

[Cristian Fogel [:cfogel]]

Verified with 79.0b2 and 80.a1(2020-07-01) on Windows 10/macOS 10.15.5