refactor(Canvas): _setupCurrentTransform => setupCurrentTransform fixing control mousedown unexpected behavior

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(): setupCurrentTransform/Control mousedown unexpected behavior [#9842](https://github.com/fabricjs/fabric.js/pull/9842)
 - ci(): Add Jest coverage to the report [#9836](https://github.com/fabricjs/fabric.js/pull/9836)
 - test(): Add cursor animation testing and migrate some easy one to jest [#9829](https://github.com/fabricjs/fabric.js/pull/9829)
 - fix(Group, Controls): Fix interactive group actions when negative scaling is involved [#9811](https://github.com/fabricjs/fabric.js/pull/9811)

src/canvas/Canvas.ts

@@ -1049,24 +1049,44 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
       if (target.selectable && target.activeOn === 'down') {
         this.setActiveObject(target, e);
       }
-      const handle = target.findControl(
-        this.getViewportPoint(e),
-        isTouchEvent(e)
-      );
-      if (target === this._activeObject && (handle || !grouped)) {
-        this._setupCurrentTransform(e, target, alreadySelected);
-        const control = handle ? handle.control : undefined,
-          pointer = this.getScenePoint(e),
-          mouseDownHandler =
-            control && control.getMouseDownHandler(e, target, control);
-        mouseDownHandler &&
+      const isSelected = target === this._activeObject;
+      const controlContext =
+        isSelected &&
+        alreadySelected &&
+        target.findControl(this.getViewportPoint(e), isTouchEvent(e));
+      const transformContext =
+        controlContext ||
+        (isSelected && !grouped && ({ key: 'drag' } as const));
+
+      if (transformContext) {
+        const transform = this.setupCurrentTransform(
+          e,
+          target,
+          transformContext
+        );
+        this.fire('before:transform', {
+          e,
+          transform,
+        });
+
+        const mouseDownHandler =
+          controlContext &&
+          controlContext.control.getMouseDownHandler(
+            e,
+            target,
+            controlContext.control
+          );
+
+        if (mouseDownHandler) {
+          const pointer = this.getScenePoint(e);
           mouseDownHandler.call(
-            control,
+            controlContext.control,
             e,
-            this._currentTransform!,
+            transform,
             pointer.x,
             pointer.y
           );
+        }
       }
     }
     //  we clear `_objectsToRender` in case of a change in order to repopulate it at rendering

src/canvas/SelectableCanvas.spec.ts

@@ -2,6 +2,7 @@ import { FabricObject } from '../shapes/Object/FabricObject';
 import { Point } from '../Point';
 import { Canvas } from './Canvas';
 import { Group } from '../shapes/Group';
+import { Rect } from '../shapes/Rect';
 
 describe('Selectable Canvas', () => {
   describe('_pointIsInObjectSelectionArea', () => {
@@ -460,7 +461,7 @@ describe('Selectable Canvas', () => {
       canvas.on('before:transform', spy);
       const setupCurrentTransformSpy = jest.spyOn(
         canvas,
-        '_setupCurrentTransform'
+        'setupCurrentTransform'
       );
 
       const {
@@ -478,7 +479,85 @@ describe('Selectable Canvas', () => {
       expect(canvas._currentTransform).toBeDefined();
       expect(canvas._currentTransform).toHaveProperty('target', object);
       expect(canvas._currentTransform).toHaveProperty('corner', controlKey);
+      expect(setupCurrentTransformSpy).toHaveReturnedWith(
+        canvas._currentTransform
+      );
       expect(spy).toHaveBeenCalledTimes(1);
     });
+
+    describe('setting current transform', () => {
+      const rectOptions = {
+        left: 75,
+        top: 75,
+        width: 50,
+        height: 50,
+      };
+      const eAt100 = new MouseEvent('mousedown', {
+        clientX: 100,
+        clientY: 100,
+      });
+      const eOverTLControl = new MouseEvent('mousedown', {
+        clientX: 69,
+        clientY: 69,
+      });
+      const eOverMLControl = new MouseEvent('mousedown', {
+        clientX: 69,
+        clientY: 95,
+        shiftKey: true,
+      });
+
+      test.each([
+        { e: eAt100, name: '100x100' },
+        { e: eOverTLControl, name: 'over TL' },
+        { e: eOverMLControl, name: 'over skew ML' },
+      ])('event $name', ({ e }) => {
+        const canvas = new Canvas();
+        const rect = new Rect(rectOptions);
+        jest.spyOn(rect, 'toJSON').mockReturnValue('rect');
+        canvas.add(rect);
+        canvas.setActiveObject(rect);
+        expect(
+          canvas.setupCurrentTransform(
+            e,
+            rect,
+            rect.findControl(canvas.getViewportPoint(e)) || {
+              key: 'drag',
+            }
+          )
+        ).toMatchSnapshot();
+      });
+
+      it('control mousedown handler should be called only if current transform uses it', () => {
+        const canvas = new Canvas();
+        const rect = new Rect(rectOptions);
+        jest.spyOn(rect, 'toJSON').mockReturnValue('rect');
+        canvas.add(rect);
+        canvas.setActiveObject(rect);
+        const hit = rect.findControl(canvas.getViewportPoint(eOverTLControl));
+
+        expect(hit).toBeDefined();
+        expect(hit?.key).toBe('tl');
+        const controlDownSpy = jest.spyOn(hit!.control, 'getMouseDownHandler');
+        const spy = jest.spyOn(canvas, 'setupCurrentTransform');
+        jest.spyOn(canvas, 'findTarget').mockReturnValue(rect);
+
+        canvas.getSelectionElement().dispatchEvent(eOverTLControl);
+        expect(spy).toHaveBeenCalledTimes(1);
+        expect(spy).toHaveBeenCalledWith(eOverTLControl, rect, hit);
+        expect(controlDownSpy).toHaveBeenCalledTimes(1);
+
+        spy.mockClear();
+        controlDownSpy.mockClear();
+
+        canvas.discardActiveObject();
+
+        canvas.getSelectionElement().dispatchEvent(eOverTLControl);
+        expect(spy).toHaveBeenCalledTimes(1);
+        expect(spy).toHaveBeenCalledWith(eOverTLControl, rect, {
+          key: 'drag',
+        });
+        expect(controlDownSpy).not.toHaveBeenCalled();
+      });
+    });
   });
 });

src/canvas/SelectableCanvas.ts

@@ -1,5 +1,4 @@
 import { dragHandler } from '../controls/drag';
-import { getActionFromCorner } from '../controls/util';
 import { Point } from '../Point';
 import { FabricObject } from '../shapes/Object/FabricObject';
 import type {
@@ -37,6 +36,7 @@ import type { CanvasOptions } from './CanvasOptions';
 import { canvasDefaults } from './CanvasOptions';
 import { Intersection } from '../Intersection';
 import { isActiveSelection } from '../util/typeAssertions';
+import type { Control } from '../controls/Control';
 
 /**
  * Canvas class
@@ -568,16 +568,31 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   }
 
   /**
-   * @private
+   * Setup state used by the canvas to execute object transforms via the control API
    * @param {Event} e Event object
    * @param {FabricObject} target
-   * @param {boolean} [alreadySelected] pass true to setup the active control
+   * @param [transformContext] pass {@link FabricObject.findControl}, `{ key: 'drag' }` to setup dragging
+   * or undefined to block transforming
    */
-  _setupCurrentTransform(
+  setupCurrentTransform(
     e: TPointerEvent,
     target: FabricObject,
-    alreadySelected: boolean
-  ): void {
+    transformContext?:
+      | { key: string; control: Control }
+      | { key: 'drag'; control?: never }
+  ): Transform {
+    const key = transformContext?.key || '';
+    const control = transformContext?.control;
+    const action = control?.getActionName(e, control, target) ?? key,
+      altKey = e[this.centeredKey as ModifierKey],
+      origin = this._shouldCenterTransform(target, action, altKey)
+        ? ({ x: CENTER, y: CENTER } as const)
+        : this._getOriginFromCorner(target, key);
+
+    /**
+     * relative to target's containing coordinate plane
+     * both agree on every point
+     **/
     const pointer = target.group
       ? // transform pointer to target's containing coordinate plane
         sendPointToPlane(
@@ -586,56 +601,63 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
           target.group.calcTransformMatrix()
         )
       : this.getScenePoint(e);
-    const { key: corner = '', control } = target.getActiveControl() || {},
-      actionHandler =
-        alreadySelected && control
-          ? control.getActionHandler(e, target, control)?.bind(control)
-          : dragHandler,
-      action = getActionFromCorner(alreadySelected, corner, e, target),
-      altKey = e[this.centeredKey as ModifierKey],
-      origin = this._shouldCenterTransform(target, action, altKey)
-        ? ({ x: CENTER, y: CENTER } as const)
-        : this._getOriginFromCorner(target, corner),
-      /**
-       * relative to target's containing coordinate plane
-       * both agree on every point
-       **/
-      transform: Transform = {
-        target: target,
-        action,
-        actionHandler,
-        actionPerformed: false,
-        corner,
-        scaleX: target.scaleX,
-        scaleY: target.scaleY,
-        skewX: target.skewX,
-        skewY: target.skewY,
-        offsetX: pointer.x - target.left,
-        offsetY: pointer.y - target.top,
+    const transform: Transform = {
+      target: target,
+      action,
+      actionHandler: control
+        ? control.getActionHandler(e, target, control)?.bind(control)
+        : key === 'drag'
+        ? dragHandler
+        : undefined,
+      actionPerformed: false,
+      corner: control ? key : '',
+      scaleX: target.scaleX,
+      scaleY: target.scaleY,
+      skewX: target.skewX,
+      skewY: target.skewY,
+      offsetX: pointer.x - target.left,
+      offsetY: pointer.y - target.top,
+      originX: origin.x,
+      originY: origin.y,
+      ex: pointer.x,
+      ey: pointer.y,
+      lastX: pointer.x,
+      lastY: pointer.y,
+      theta: degreesToRadians(target.angle),
+      width: target.width,
+      height: target.height,
+      shiftKey: e.shiftKey,
+      altKey,
+      original: {
+        ...saveObjectTransform(target),
         originX: origin.x,
         originY: origin.y,
-        ex: pointer.x,
-        ey: pointer.y,
-        lastX: pointer.x,
-        lastY: pointer.y,
-        theta: degreesToRadians(target.angle),
-        width: target.width,
-        height: target.height,
-        shiftKey: e.shiftKey,
-        altKey,
-        original: {
-          ...saveObjectTransform(target),
-          originX: origin.x,
-          originY: origin.y,
-        },
-      };
+      },
+    };
 
-    this._currentTransform = transform;
+    return (this._currentTransform = transform);
+  }
 
-    this.fire('before:transform', {
+  /**
+   * @deprecated use {@link setupCurrentTransform} and fire your own `before:transform` event if you need to
+   * @param {Event} e Event object
+   * @param {FabricObject} target
+   * @param {boolean} [activateControl] pass true to setup the active control
+   */
+  _setupCurrentTransform(
+    e: TPointerEvent,
+    target: FabricObject,
+    activateControl: boolean
+  ) {
+    const transform = this.setupCurrentTransform(
       e,
-      transform,
-    });
+      target,
+      (activateControl &&
+        target.findControl(this.getViewportPoint(e), isTouchEvent(e))) || {
+        key: 'drag',
+      }
+    );
+    this.fire('before:transform', { e, transform });
   }
 
   /**