fix(Canvas): Fix searchPossibleTargets for non-interactive nested tar…

…gets (#9762)

Co-authored-by: Andrea Bogazzi <[email protected]>
Co-authored-by: Shachar <[email protected]>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(Canvas): Fix searchPossibleTargets for non-interactive nested targets [#9762](https://github.com/fabricjs/fabric.js/pull/9762)
 - test(): Rename svg tests [#9775](https://github.com/fabricjs/fabric.js/pull/9775)
 - refactor(): `_findTargetCorner` is now called `findControl` and returns the key and the control and the coordinates [#9668](https://github.com/fabricjs/fabric.js/pull/9668)
 - feat(LayoutManager): Handle the case of activeSelection with objects inside different groups [#9651](https://github.com/fabricjs/fabric.js/pull/9651)

src/canvas/SelectableCanvas.spec.ts

@@ -303,6 +303,137 @@ describe('Selectable Canvas', () => {
     });
   });
 
+  describe('searchPossibleTargets', () => {
+    test('the target returned will stop at the first non interactive container', () => {
+      const object = new FabricObject({
+        id: 'a',
+        left: 0,
+        top: 0,
+        width: 10,
+        height: 10,
+        padding: 0,
+        strokeWidth: 0,
+      });
+      const groupB = new Group([object], {
+        id: 'b',
+        interactive: true,
+        subTargetCheck: true,
+      });
+      const groupC = new Group([groupB], {
+        id: 'c',
+        interactive: false,
+        subTargetCheck: true,
+      });
+      const groupD = new Group([groupC], {
+        id: 'd',
+        interactive: true,
+        subTargetCheck: true,
+      });
+      const canvas = new Canvas(undefined, { renderOnAddRemove: false });
+      canvas.add(groupD);
+      const target = canvas.searchPossibleTargets(
+        canvas.getObjects(),
+        groupD.getCenterPoint()
+      );
+      expect(target).toBe(groupC);
+      expect(canvas.targets.map((obj) => obj.id)).toEqual(['a', 'b', 'c']);
+    });
+    test('a interactive group covered by a non interactive group wont be selected', () => {
+      const object = new FabricObject({
+        id: 'a',
+        left: 0,
+        top: 0,
+        width: 10,
+        height: 10,
+        padding: 0,
+        strokeWidth: 0,
+      });
+      const groupB = new Group([object], {
+        id: 'b',
+        interactive: true,
+        subTargetCheck: true,
+      });
+      const groupC = new Group([groupB], {
+        id: 'c',
+        interactive: false,
+        subTargetCheck: true,
+      });
+      const groupD = new Group([groupC], {
+        id: 'd',
+        interactive: true,
+        subTargetCheck: true,
+      });
+      const groupE = new Group([groupD], {
+        id: 'e',
+        interactive: true,
+        subTargetCheck: true,
+      });
+      const canvas = new Canvas(undefined, { renderOnAddRemove: false });
+      canvas.add(groupE);
+      const target = canvas.searchPossibleTargets(
+        canvas.getObjects(),
+        groupD.getCenterPoint()
+      );
+      expect(target).toBe(groupC);
+      expect(canvas.targets.map((obj) => obj.id)).toEqual(['a', 'b', 'c', 'd']);
+    });
+
+    test('nested non interactive groups with subTargetCheck', () => {
+      const object = new FabricObject({
+        left: 0,
+        top: 0,
+        width: 10,
+        height: 10,
+        padding: 0,
+        strokeWidth: 0,
+      });
+
+      const object2 = new FabricObject({
+        left: 20,
+        top: 0,
+        width: 10,
+        height: 10,
+        padding: 0,
+        strokeWidth: 0,
+      });
+
+      const object3 = new FabricObject({
+        left: 40,
+        top: 0,
+        width: 10,
+        height: 10,
+        padding: 0,
+        strokeWidth: 0,
+      });
+
+      const nestedGroup = new Group([object2, object3], {
+        interactive: false,
+        subTargetCheck: true,
+      });
+
+      const canvas = new Canvas(undefined, { renderOnAddRemove: false });
+      const group = new Group([object, nestedGroup], {
+        interactive: true,
+        subTargetCheck: true,
+      });
+      canvas.add(group);
+
+      const object2Position = object2.getCenterPoint();
+      const target = canvas.searchPossibleTargets(
+        canvas.getObjects(),
+        object2Position
+      );
+      expect(target).toBe(nestedGroup);
+
+      nestedGroup.set({ interactive: true });
+      const nestedTarget = canvas.searchPossibleTargets(
+        canvas.getObjects(),
+        object2Position
+      );
+      expect(nestedTarget).toBe(object2);
+    });
+  });
+
   describe('setupCurrentTransform', () => {
     test.each(
       ['tl', 'mt', 'tr', 'mr', 'br', 'mb', 'bl', 'ml', 'mtr']

src/canvas/SelectableCanvas.ts

@@ -188,7 +188,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   declare fireMiddleClick: boolean;
 
   /**
-   * Keep track of the subTargets for Mouse Events
+   * Keep track of the subTargets for Mouse Events, ordered bottom up from innermost nested subTarget
    * @type FabricObject[]
    */
   targets: FabricObject[] = [];
@@ -860,15 +860,31 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
     pointer: Point
   ): FabricObject | undefined {
     const target = this._searchPossibleTargets(objects, pointer);
-    // if we found something in this.targets, and the group is interactive, return that subTarget
+
+    // if we found something in this.targets, and the group is interactive, return the innermost subTarget
+    // that is still interactive
     // TODO: reverify why interactive. the target should be returned always, but selected only
     // if interactive.
-    return target &&
+    if (
+      target &&
       isCollection(target) &&
       target.interactive &&
       this.targets[0]
-      ? this.targets[0]
-      : target;
+    ) {
+      /** targets[0] is the innermost nested target, but it could be inside non interactive groups and so not a selection target */
+      const targets = this.targets;
+      for (let i = targets.length - 1; i > 0; i--) {
+        const t = targets[i];
+        if (!(isCollection(t) && t.interactive)) {
+          // one of the subtargets was not interactive. that is the last subtarget we can return.
+          // we can't dig more deep;
+          return t;
+        }
+      }
+      return targets[0];
+    }
+
+    return target;
   }
 
   /**