1451353 - The Mouse cursor is displayed as a closed hand when on top of the "Move" highlighter point [Shape Tool]

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Rares Doghi, Desktop QA]

[Affected versions]: 
Nightly 61.0a1

[Affected platforms]:
Platforms: Ubuntu 16.04 x64.

[Steps to reproduce]:
1. Go to about:config and set "layout.css.shape-outside.enabled" to true.
2. Go to http://labs.jensimmons.com/2016/examples/shapes-3.html
3. Right click on the image and then click on "Inspect Element".
4. Ctrl + Click on the button displayed next to clip-path to toggle the shape tool on
5. Click on the middle point from the Bottom(the one that Moves the shape)

[Expected result]:
- The Mouse cursor should change its state to an Arrow with 4 directions when on top of "MOVE" highlighter point. 
If the Arrow with 4 directions is not available on Linux it should be displayed as an Open hand before the user clicks the Point.

[Actual result]:
- The Mouse cursor is displayed as a closed hand when on top of the "MOVE" point and it will not change its state when clicking and dragging the point

Comment 1

[Patrick Brosset <:pbro>]

Just a few clarifications about the steps to reproduce:
- This is a Linux only bug.
- You should Ctrl+Click on the icon next to the *shape-outside* property (not clip-path).
- You should position your mouse over the center point.

This is likely a bug where the move cursor simply does not exist on Ubuntu. 
In which case, as Rares said, we should switch to using the hand cursor, and make sure it switches to the closed hand cursor while dragging.

This is a good first bug for someone who's interested in getting started with contributing to DevTools.
We have contribution docs here: http://docs.firefox-dev.tools/
It probably will be a CSS-only fix. The CSS code for this is in /devtools/server/actors/highlighters.css (code starts after the /* Shapes highlighter */ section).
The JavaScript code for the shapes tool is in /devtools/server/actors/highlighters/shapes.js

Razvan Caliman is currently the person who knows the most about this code, so I have marked him as a mentor on this bug.
If you are interested in fixing this, feel free to use the needinfo? flag to ask questions and either him or I can help you get started!

Comment 2

[Whitney Meulink]

Hi there, I am a student at a University working with a team of 3 other students who are also gaining experience in Open Source. We thought that this bug would be a good first bug to get familiar with the Patch process.

Comment 3

[Razvan Caliman [:rcaliman]]

Hi, Whitney! Thanks for showing interest in this bug. It is indeed a good first bug to get accustomed to the process.

The link which Patrick posted above is a good reference for getting with the code and setting up the system. Let me know if you need help with that.

This bug is likely impacting just Linux. It was tested and it works as expected on MacOS. The place you should look into is not the CSS file (cursors are not assigned there) but in /devtools/server/actors/highlighters/shapes.js

Check out the _handleMarkerHover(point) method:
https://searchfox.org/mozilla-central/source/devtools/server/actors/highlighters/shapes.js#1361

It gets called on "mousemove" with a `point` argument that matches the hovered marker in the editor. Assigning of the appropriate cursor for markers for resizing and scaling is done a bit lower in that file, around line 1403 
https://searchfox.org/mozilla-central/source/devtools/server/actors/highlighters/shapes.js#1403

this.setCursor(`${direction}-resize`);

It's possible that some of those cursors don't exist on Linux. There's a list of all named cursors valid in CSS:
https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

Let me know if you need other guidance on this.

Comment 4

[Whitney Meulink]

Hi there,

We have determined that the cursor "move" does not exist in Linux. We have changed the cursor to Grab on Hover and Grabbing on Click for the middle point from the bottom (The one that moves the shape). We are concerned that this will also be changed on MacOS as well, we are wondering if this is okay or do you want the MacOS behavior untouched for now? We thought we would ask before submitting the pull request. 

Thank you for your time!

Comment 5

[Razvan Caliman [:rcaliman]]

Hi Whitney, 

Thanks for solving the issue for Linux. The behaviour for MacOS / Windows should remain untouched because those systems have the expanded set of cursors.

Comment 6

[Whitney Meulink]

Attachment: Bug 1451353 - Changed move to hoverCursor to allow the hover animation to go from grab to grabbing since move does not exist in Linux. .

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

Comment 7

[Razvan Caliman [:rcaliman]]

Comment on attachment 8981589 [details]
Bug 1451353 - Changed move to hoverCursor to allow the hover animation to go from grab to grabbing since move does not exist in Linux. .

https://reviewboard.mozilla.org/r/247688/#review253976

This changes the cursor for all platforms, not just Linux, and I'm ok with that because it's consistent and still within UX expectations. 
The added complexity in checking this cursor per OS would not be justified in this scenario. I approve of this simple solution.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bc5d7687da9
Changed move to hoverCursor to allow the hover animation to go from grab to grabbing since move does not exist in Linux. r=rcaliman.

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5bc5d7687da9

Comment 10

[David Olah]

I retested the issue on Ubuntu and now, for ctr+click on Circle and Ellipse (Ex: http://labs.jensimmons.com/2016/examples/shapes-1.html ), the mouse cursor changes from open hand to close hand only when it is dragged.

Steps:
1) Go to link from example
2) Inspect element and Ctrl + Click the shapes highlighter
3) Hover the mouse on the move point (it is an open hand)
4) Click the move point

Actual result: The cursor doesn't change to closed hand only when the cursor is dragged

Expected result: The cursor changes to closed hand in the moment that you click on the move point

Comment 11

[Andreea Pavel [:apavel]]

(In reply to David Olah from comment #10)
> I retested the issue on Ubuntu and now, for ctr+click on Circle and Ellipse
> (Ex: http://labs.jensimmons.com/2016/examples/shapes-1.html ), the mouse
> cursor changes from open hand to close hand only when it is dragged.
> 
> Steps:
> 1) Go to link from example
> 2) Inspect element and Ctrl + Click the shapes highlighter
> 3) Hover the mouse on the move point (it is an open hand)
> 4) Click the move point
> 
> Actual result: The cursor doesn't change to closed hand only when the cursor
> is dragged
> 
> Expected result: The cursor changes to closed hand in the moment that you
> click on the move point

Hi, tested this and indeed the actual result is that the cursor doesn't change to a closed hand except when it's dragged. Let me know if there is anything else I can help with.

Comment 12

[Rares Doghi, Desktop QA]

Hi, I retested this issue in version 62.0a1 (2018-06-24) , and the issue no longer occurs. i will mark it Verified as fixed.