fix(Canvas): setActiveObject should update canvas#_activeSelection

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Changelog
 
+- fix(Canvas): `setActiveObject` should update `canvas#_activeSelection` [#9336](https://github.com/fabricjs/fabric.js/pull/9336)
+
 ## [next]
 
 - fix(StaticCanvas): disposing animations [#9361](https://github.com/fabricjs/fabric.js/pull/9361)

src/canvas/SelectableCanvas.ts

@@ -293,7 +293,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   protected declare _isCurrentlyDrawing: boolean;
   declare freeDrawingBrush?: BaseBrush;
   declare _activeObject?: FabricObject;
-  protected readonly _activeSelection: ActiveSelection;
+  protected _activeSelection: ActiveSelection;
 
   constructor(
     el?: string | HTMLCanvasElement,
@@ -1105,6 +1105,12 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
     }
     this._activeObject = object;
 
+    if (object instanceof ActiveSelection && this._activeSelection !== object) {
+      this._activeSelection = object;
+      object.set('canvas', this);
+      object.setCoords();
+    }
+
     return true;
   }
 

src/shapes/ActiveSelection.spec.ts

@@ -1,5 +1,6 @@
 import { Canvas } from '../canvas/Canvas';
 import { ActiveSelection } from './ActiveSelection';
+import { Group } from './Group';
 import { FabricObject } from './Object/FabricObject';
 
 describe('ActiveSelection', () => {
@@ -86,4 +87,37 @@ describe('ActiveSelection', () => {
     expect(canvas.getActiveSelection().lineCoords).toMatchSnapshot();
     expect(canvas.getActiveSelection().aCoords).toMatchSnapshot();
   });
+
+  it('`setActiveObject` should update the active selection ref on canvas if it changed', () => {
+    const canvas = new Canvas(null);
+    const obj1 = new FabricObject();
+    const obj2 = new FabricObject();
+    canvas.add(obj1, obj2);
+    const activeSelection = new ActiveSelection([obj1, obj2]);
+    const spy = jest.spyOn(activeSelection, 'setCoords');
+    canvas.setActiveObject(activeSelection);
+    expect(canvas.getActiveSelection()).toBe(activeSelection);
+    expect(canvas.getActiveObjects()).toEqual([obj1, obj2]);
+    expect(spy).toHaveBeenCalled();
+    expect(activeSelection.canvas).toBe(canvas);
+
+    spy.mockClear();
+    canvas.setActiveObject(activeSelection);
+    expect(spy).not.toHaveBeenCalled();
+  });
+
+  it('transferring an object between active selections keeps its owning group', () => {
+    const object = new FabricObject();
+    const group = new Group([object]);
+    const activeSelection1 = new ActiveSelection([object]);
+    const activeSelection2 = new ActiveSelection();
+    expect(object.group).toBe(activeSelection1);
+    expect(object.getParent(true)).toBe(group);
+    activeSelection2.add(object);
+    expect(object.group).toBe(activeSelection2);
+    expect(object.getParent(true)).toBe(group);
+    activeSelection2.removeAll();
+    expect(object.group).toBe(group);
+    expect(object.getParent(true)).toBe(group);
+  });
 });

src/shapes/ActiveSelection.ts

@@ -86,7 +86,9 @@ export class ActiveSelection extends Group {
       //  save ref to group for later in order to return to it
       const parent = object.group;
       parent._exitGroup(object);
-      object.__owningGroup = parent;
+      // make sure we are setting the correct owning group
+      // in case `object` is transferred between active selections
+      !(parent instanceof ActiveSelection) && (object.__owningGroup = parent);
     }
     this._enterGroup(object, removeParentTransform);
     return true;