Moving a node with keyboard crashes when scrollbar appears on its container

[lredor]

Steps to reproduce:

• Import "KeyMovePb" project from KeyMovePbProject.zip
• Open the diagram "diagramWithContainer"
• Select "C1"
• Use the right arrow key to move this node to the right
• The first moves are OK.
• But as soon as the scroll bar appears on "subPackage", pressing the right arrow key no longer has any effect.
• If you press on the left arrow key, only the first press has effect. The scrollbar disappears, and the next pressing has no effect.

The video "movingRightElevenTimesThenMovingThreeTimes.mkv" in this archive replays this scenario.

[lredor]

Here are some technical analyses :

• At each key press, NoCopyDragEditPartsTrackerEx.placeMouseInViewer() is called twice:
  • One for org.eclipse.gmf.runtime.diagram.ui.tools.DragEditPartsTrackerEx.handleKeyDown(KeyEvent) line 296
  • and one for org.eclipse.gef.tools.DragEditPartsTracker.handleKeyDown(KeyEvent) line 461
• In 2007, some changes have been done in GEF to allow GMF to override the GEF behavior concerning the moving with arrow keys (bugzilla 213359 - Make GEF's snapping and tools extensible to allow moving shapes with arrow keys and commit ecda0d3b. These changes have been done for bugzilla 111901- Ability to move shapes using cursors keys. (by default). If I remove the GMF override, ie in org.eclipse.gmf.runtime.diagram.ui.tools.DragEditPartsTrackerEx.handleKeyDown(KeyEvent) only call super.handleKeyDown(e), I have no longer the problem. But I observed the problem described by Cherie Revells in the last comment of bug 213359.
• At handleKeyDown call, org.eclipse.gef.tools.AbstractTool.accStepIncrement() computes the increment according to the duration of the key press (from 1 to 16 to have an acceleration of the move). This can explain a different behavior according the duration of the key press (first gap is from one to four).

Globally, the current GEF/GMF implementation does not allow solving the problem. This implementation is based on a simulation of a mouse move when one of the arrow key is pressed. But this implementation mixes absolutes GMF coordinates and mouse cursor coordinates that uses "screen" coordinates (for example in org.eclipse.gef.tools.AbstractTool.placeMouseInViewer(Point) by using org.eclipse.swt.widgets.Control.toDisplay(Point)). This way of doing things can not handle the case where a container has a scrollbar. Indeed, in this case, the "absolute coordinates" returned by Draw2D do not reflect the reals coordinates and can not be used to set the "mouse location".

Following these analyses, we decided to implement a new strategy that does not rely on mouse movement simulation. But that directly creates the request based on the arrow key pressed.

[lredor]

Ideally this ticket would have required changes in GEF and GMF. For the moment, the change has been made directly in Sirius. Issues have been created to integrate the changes in GEF/GMF into future releases:

• New GEF APIs :
  • Ticket GEF #426 - Make AbstractTool's accessibleStep mechanism accessible for subclasses : Closed and will be in next release release 4.20 for Eclipse 2024-06.
  • Ticket GMF #62 - Use GEF version 3.20
  • Ticket Sirius #376 - Clean code done for #359 by using new version of GMF/GEF: A PR is already available, #377 (but could be merged only when a GEF/GMF bump has been done on Sirius side).

• Small GMF bug :
  • Ticket GMF #63 - Use precise coordinates in DragEditPartsTrackerEx.snapPoint with corresponding PR #64
  • Ticket Sirius #378 - Clean code done for #359 by using new version of GMF : A PR is already available, #379 (but could be merged only when a GMF bump has been done on Sirius side).

[lredor]

Known limitations:

Technically the behavior has been changed: The press to an arrow key now directly handle a "move request" to the concerned edit parts and the corresponding SetBoundsCommand are executed. But during the analysis of this issue some behaviors, that can be considered as limitations, have been detected. They are not in the scope of this issue:

• A- The result of a "long" press to left (or top) arrow key on a node in a container, is that anchors of the node are visible in a "not visible" area. This behavior already existed before. Result in images:
  • Initial state: The node C1 is selected.
    
  • Result after long left key press: The diagram has been scrolled (and not the container)
    
  • Result after selecting the diagram: The container subPackage has a "long" scrollbar and a manual scroll is necessary to reveal the node C1.
    
• The grid is "fixed" on the diagram. If container has scrollbars, the "snap to grid" behavior can be sometimes strange.
  • B- The alignment is "true" for the current state of the container scrollbars but becomes "false" as soon as the scrollbar is moved. Result in images:
    • Initial state: The node C1 is selected. Its container has a vertical scrollbar as it contains another node lower down.
      
    • After a move to the right and a move to the bottom: The node C1 is aligned on horizontal and vertical grid.
      
    • After moving scrollbar: If the scrollbar is moved down, C1 is no longer aligned to the vertical grid.
      
  • C- According to the size of the moved node and the size of the grid step, the behavior can be strange when the node is moved "out of the current limits" of the node. The node seems not aligned to the grid and the following moves will not be as big as the step defined by the grid. Result in images:
    • Initial state: The node C1 is selected.
      
    • After a first move to the right: Everything is OK, the node C1 is aligned on the horizontal grid.
      
    • After a second move to the right: The node C1 seems not aligned to the grid. If we break down what happened:
      • The node has been moved and aligned on the grid,
      • a scrollbar appeared on the container because it was not big enough,
      • the scrollbar has been moved to reveal the node,
      • and so we fall back into the case above where the alignment on the grid is "temporarily" true.
        
    • After moving the scrollbar to its origin, we can see that the node is aligned to the grid.
      
    • If another move is done just after the second move, the move delta will not be a grid step as you can see on the following image.