1451683 - The "resizing" cursor isn't changed back to a normal cursor after resizing the subpanel

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Julien Wajsberg [:julienw]]

STR:
1. Open the inspector on any page.
2. Resize the sidebar.

=> Observe the cursor isn't back to normal when hovering empty spaces in the sidebar.
This is especially visible with the Layout or Font subpanels because there are no elements overriding the cursor in these panels.

Comment 1

[Daniel Bodea [:danibodea]]

See also: bug 1451666. This issue was fixed for the animations tab, but the others tabs still reproduce the issue.

Comment 2

[Patrick Brosset <:pbro>]

Looks like this was fixed in the animation tab by forcing the default cursor there, but really I think the fix would be to find the root cause for this and fix it there.
My fut feeling so far is that this has to do with the SplitBox.js component:

  onStartMove() {
    const splitBox = ReactDOM.findDOMNode(this);
    const doc = splitBox.ownerDocument;
    let defaultCursor = doc.documentElement.style.cursor;
    doc.documentElement.style.cursor = (this.state.vert ? "ew-resize" : "ns-resize");

    splitBox.classList.add("dragging");

    this.setState({
      defaultCursor: defaultCursor
    });
  }

Somehow, it probably never resets the right cursor when the user stops moving.

Comment 3

[Patrick Brosset <:pbro>]

Looks like this is called twice in a row. The first time the code remembers the cursor (the default), then changes it to ew-resize (or ns-resize). But then the function gets called again for some reason, and that ends up storing ew-resize as the default cursor.

Comment 4

[Patrick Brosset <:pbro>]

I don't know why this is called twice. The caller is mousedown event listener in devtools/client/shared/components/splitter/Draggable.js.
If I log from that handler, I can see the log twice, with the same event target DOM element.

So just adding some local state like this.isDragging and setting it to true after the event listener was called the first time will fix the issue.

It would be nice to know when this started. I can reproduce it in Firefox 60 but not in 59. I may attempt to run mozregression to see if there is a better solution.

Comment 5

[Patrick Brosset <:pbro>]

Mozregression says:

Last good revision: ff4c0d600d07c9a5e0fe503a326db767962ea052
First bad revision: d17b9126dca6bb1f04a9bc454367159c7c10c861
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ff4c0d600d07c9a5e0fe503a326db767962ea052&tochange=d17b9126dca6bb1f04a9bc454367159c7c10c861

Which means this was regressed by bug 1420130, which is related to when we migrated to React 16.

Comment 6

[Patrick Brosset <:pbro>]

I talked to Mike Ratcliffe who said this is a XUL-related bug that would need to be fixed. Bug 1342297 I believe.
In the meantime, doing the fix described in comment 4 would work.

Comment 7

[Patrick Brosset <:pbro>]

Attachment: Bug 1451683 - Avoid reacting to mousedown twice in a row when starting to drag the splitter;

Review commit: https://reviewboard.mozilla.org/r/241722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241722/

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8973245 [details]
Bug 1451683 - Avoid reacting to mousedown twice in a row when starting to drag the splitter;

https://reviewboard.mozilla.org/r/241722/#review248458

Comment 9

[Patrick Brosset <:pbro>]

Thanks Mike.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=52e03668baa3b17e2cbbaa24d91a345c703f2f54
Landing this now in 62.
I'll request uplift to 61 later.

Comment 10

[Patrick Brosset <:pbro>]

It would be great if someone could verify the fix once it's in nightly.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3024ee56e9db
Avoid reacting to mousedown twice in a row when starting to drag the splitter; r=miker

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3024ee56e9db

Comment 13

[Daniel Bodea [:danibodea]]

Hi,

I have verified this fix on Windows 10 x64, Linux 16.04 LTS x64 and Mac OS X 10.12.6.
The described issue could not be reproduced anymore on any of the OSes.
Verified fix in target version 62.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this. It grafts cleanly as-landed.

Comment 15

[Patrick Brosset <:pbro>]

Comment on attachment 8973245 [details]
Bug 1451683 - Avoid reacting to mousedown twice in a row when starting to drag the splitter;

Approval Request Comment
[Feature/Bug causing the regression]: 1420130
[User impact if declined]: If declined, users of the inspector panel in DevTools may see a resizing mouse cursor rather than a default mouse cursor, in some situations.
[Is this code covered by automated tests?]: No, this does not impact DevTools itself apart from the cursor being stuck to the resizing icon, so I didn't add a test for this.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: Already tested and verified in nightly, I don't think we need to test again in beta. The fix was made to a piece of code that hasn't changed between those 2 channels.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Very self-contained fix, to just one DevTools module. Only impacts the mouse cursor.
[String changes made/needed]: None

Comment 16

[Julien Wajsberg [:julienw]]

(In reply to Patrick Brosset <:pbro> from comment #10)
> It would be great if someone could verify the fix once it's in nightly.

The original issue I had is fixed in latest nightly.
Thanks a lot !

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8973245 [details]
Bug 1451683 - Avoid reacting to mousedown twice in a row when starting to drag the splitter;

Avoid situations where we sometimes end up with the wrong mouse cursor while using devtools. Approved for 61.0b5.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/983cb96d1a3a

Comment 19

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 61.0a1 (2018-04-05) on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 61.0b5 on Windows 10 x64, mac OS 10.12.6 and Ubuntu 14.04 LTS.