Closed Bug 1451683 Opened 2 years ago Closed Last year

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

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 wontfix, firefox60 wontfix, firefox61 verified, firefox62 verified)

VERIFIED FIXED
Firefox 62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- verified
firefox62 --- verified

People

(Reporter: julienw, Assigned: pbro)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
See also: bug 1451666. This issue was fixed for the animations tab, but the others tabs still reproduce the issue.
See Also: → 1451666
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.
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.
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.
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.
Blocks: 1420130
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.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Priority: -- → P3
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
Attachment #8973245 - Flags: review?(mratcliffe) → review+
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.
It would be great if someone could verify the fix once it's in nightly.
Flags: qe-verify+
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3024ee56e9db
Avoid reacting to mousedown twice in a row when starting to drag the splitter; r=miker
https://hg.mozilla.org/mozilla-central/rev/3024ee56e9db
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
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.
Status: RESOLVED → VERIFIED
Please request Beta approval on this. It grafts cleanly as-landed.
Flags: needinfo?(pbrosset)
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
Flags: needinfo?(pbrosset)
Attachment #8973245 - Flags: approval-mozilla-beta?
(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 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.
Attachment #8973245 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Flags: qe-verify+
Duplicate of this bug: 1452612
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.