fix(): searchPossibleTargets targets value

CHANGELOG.md

@@ -146,6 +146,9 @@
 - fix(Utils): fix exported svg color [#9408](https://github.com/fabricjs/fabric.js/pull/9408)
 
 ## [6.0.0-beta13]
+- fix(): `searchPossibleTargets` `targets` value [#9343](https://github.com/fabricjs/fabric.js/pull/9343)
+
+## [6.0.0-b3]
 
 - fix(Textbox): implemente a fix for the style shifting issues on new lines [#9197](https://github.com/fabricjs/fabric.js/pull/9197)
 - Fix(Control) fix a regression in `wrap with fixed anchor`, regression from #8400 [#9326](https://github.com/fabricjs/fabric.js/pull/9326)

src/canvas/Canvas.ts

@@ -393,14 +393,13 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
    * Override at will
    */
   protected findDragTargets(e: DragEvent) {
-    this.targets = [];
-    const target = this._searchPossibleTargets(
+    const { target, targets } = this.findTargets(
       this._objects,
       this.getViewportPoint(e)
     );
     return {
       target,
-      targets: [...this.targets],
+      targets: target ? targets.slice(0, targets.indexOf(target)) : targets,
     };
   }
 
@@ -1404,10 +1403,10 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
           const pointer = this.getViewportPoint(e);
           target =
             // first search active objects for a target to remove
-            this.searchPossibleTargets(prevActiveObjects, pointer) ||
+            this.findTargets(prevActiveObjects, pointer).target ||
             //  if not found, search under active selection for a target to add
             // `prevActiveObjects` will be searched but we already know they will not be found
-            this.searchPossibleTargets(this._objects, pointer);
+            this.findTargets(this._objects, pointer).target;
           // if nothing is found bail out
           if (!target || !target.selectable) {
             return false;

src/canvas/SelectableCanvas.ts

@@ -689,11 +689,11 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   }
 
   /**
-   * Method that determines what object we are clicking on
+   * Determines what object and its sub targets {@link e} should target
    * 11/09/2018 TODO: would be cool if findTarget could discern between being a full target
    * or the outside part of the corner.
    * @param {Event} e mouse event
-   * @return {FabricObject | null} the target found
+   * @return {FabricObject | undefined} the target found
    */
   findTarget(e: TPointerEvent): FabricObject | undefined {
     if (this.skipTargetFind) {
@@ -704,45 +704,59 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
       activeObject = this._activeObject,
       aObjects = this.getActiveObjects();
 
-    this.targets = [];
-
     if (activeObject && aObjects.length >= 1) {
       if (activeObject.findControl(pointer, isTouchEvent(e))) {
         // if we hit the corner of the active object, let's return that.
         return activeObject;
-      } else if (
-        aObjects.length > 1 &&
-        // check pointer is over active selection and possibly perform `subTargetCheck`
-        this.searchPossibleTargets([activeObject], pointer)
-      ) {
+      }
+
+      // check pointer is over active selection and possibly perform `subTargetCheck`
+      const { target: selectedTarget, targets: selectedTargets } =
+        this.findTargets([activeObject], pointer);
+
+      if (selectedTarget && aObjects.length > 1) {
         // active selection does not select sub targets like normal groups
+        // remove active selection for the array
+        this.targets = selectedTargets.slice(0, -1);
         return activeObject;
-      } else if (
-        activeObject === this.searchPossibleTargets([activeObject], pointer)
-      ) {
+      } else if (activeObject === selectedTarget) {
         // active object is not an active selection
         if (!this.preserveObjectStacking) {
+          this.targets = selectedTargets.slice(
+            0,
+            selectedTargets.indexOf(activeObject)
+          );
           return activeObject;
         } else {
-          const subTargets = this.targets;
-          this.targets = [];
-          const target = this.searchPossibleTargets(this._objects, pointer);
+          const { target, targets: canvasTargets } = this.findTargets(
+            this._objects,
+            pointer
+          );
+
           if (
             e[this.altSelectionKey as ModifierKey] &&
             target &&
             target !== activeObject
           ) {
             // alt selection: select active object even though it is not the top most target
-            // restore targets
-            this.targets = subTargets;
+            this.targets = canvasTargets.slice(
+              0,
+              canvasTargets.indexOf(activeObject)
+            );
             return activeObject;
           }
+
+          this.targets = target
+            ? canvasTargets.slice(0, canvasTargets.indexOf(target))
+            : canvasTargets;
           return target;
         }
       }
     }
 
-    return this.searchPossibleTargets(this._objects, pointer);
+    const { target, targets } = this.findTargets(this._objects, pointer);
+    this.targets = target ? targets.slice(0, targets.indexOf(target)) : targets;
+    return target;
   }
 
   /**
@@ -819,72 +833,81 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   }
 
   /**
-   * Internal Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted
-   * @param {Array} [objects] objects array to look into
-   * @param {Object} [pointer] x,y object of point coordinates we want to check.
-   * @return {FabricObject} **top most object from given `objects`** that contains pointer
-   * @private
+   * Search for objects containing {@link pointer}.
+   *
+   * @param {FabricObject[]} objects objects array to look into
+   * @param {Point} pointer point canvas element plane coordinates to check
+   * @param {boolean} [param2.searchStrategy] strategy
+   * @returns {FabricObject[]} path of objects starting from **top most** object on screen.
    */
-  _searchPossibleTargets(
+  protected findTargetsTraversal(
     objects: FabricObject[],
-    pointer: Point
-  ): FabricObject | undefined {
-    // Cache all targets where their bounding box contains point.
-    let i = objects.length;
-    // Do not check for currently grouped objects, since we check the parent group itself.
-    // until we call this function specifically to search inside the activeGroup
-    while (i--) {
-      const target = objects[i];
-      if (this._checkTarget(target, pointer)) {
+    pointer: Point,
+    options: { searchStrategy: 'first-hit' | 'search-all' }
+  ): FabricObject[] {
+    const targets: FabricObject[] = [];
+    for (let index = objects.length - 1; index >= 0; index--) {
+      const target = objects[index];
+      const pointerToUse = target.group
+        ? this._normalizePointer(target.group, pointer)
+        : pointer;
+      if (this._checkTarget(pointerToUse, target, pointer)) {
         if (isCollection(target) && target.subTargetCheck) {
-          const subTarget = this._searchPossibleTargets(
-            target._objects as FabricObject[],
-            pointer
+          targets.push(
+            ...this.findTargetsTraversal(
+              target._objects as FabricObject[],
+              pointer,
+              options
+            )
           );
-          subTarget && this.targets.push(subTarget);
         }
-        return target;
+        targets.push(target);
+        if (options.searchStrategy === 'first-hit') {
+          break;
+        }
       }
     }
+    return targets;
   }
 
   /**
-   * Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted
-   * @see {@link _searchPossibleTargets}
+   * Search objects for an object containing {@link pointer}
+   * depending on the tree's configuration (`subTargetCheck`, `interactive`, `selectable`)
+   *
+   * @see {@link findTarget} and {@link findTargetsTraversal}
+   *
    * @param {FabricObject[]} [objects] objects array to look into
-   * @param {Point} [pointer] coordinates from viewport to check.
-   * @return {FabricObject} **top most object on screen** that contains pointer
+   * @param {Point} pointer viewport point
+   * @return {FabricObject} **top most selectable object on screen** that contains {@link pointer}
    */
-  searchPossibleTargets(
+  findTargets(
     objects: FabricObject[],
-    pointer: Point
-  ): FabricObject | undefined {
-    const target = this._searchPossibleTargets(objects, pointer);
-
-    // 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.
-    if (
-      target &&
-      isCollection(target) &&
-      target.interactive &&
-      this.targets[0]
-    ) {
-      /** 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];
-    }
+    pointer: Point,
+    {
+      searchStrategy = 'first-hit',
+    }: { searchStrategy?: 'first-hit' | 'search-all' } = {}
+  ) {
+    const targets = this.findTargetsTraversal(objects, pointer, {
+      searchStrategy,
+    });
 
-    return target;
+    const target = targets.find((target) => {
+      return (
+        !target.group || target.group.interactive || objects.includes(target)
+      );
+    });
+
+    return {
+      target,
+      targets,
+    };
+  }
+
+  /**
+   * @deprecated use {@link findTargets} instead
+   */
+  searchPossibleTargets(objects: FabricObject[], pointer: Point) {
+    return this.findTargets(objects, pointer).target;
   }
 
   /**

src/canvas/__tests__/__snapshots__/eventData.test.ts.snap

@@ -114,6 +114,29 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv
         "y": -13,
       },
       "currentSubTargets": [],
+      "button": 1,
+      "e": MouseEvent {
+        "isTrusted": false,
+      },
+      "isClick": false,
+      "pointer": Point {
+        "x": 50,
+        "y": 50,
+      },
+      "subTargets": [],
+      "target": "Drag Target",
+      "transform": null,
+    },
+  ],
+  [
+    "mouse:up",
+    {
+      "absolutePointer": Point {
+        "x": -30,
+        "y": -13,
+      },
+      "button": 1,
+      "currentSubTargets": [],
       "currentTarget": "Drag Target",
       "e": MouseEvent {
         "isTrusted": false,
@@ -127,7 +150,9 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "transform": null,
       "viewportPoint": Point {
@@ -228,7 +253,9 @@ exports[`Canvas event data HTML event "dragenter" should fire a corresponding ca
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "viewportPoint": Point {
         "x": 50,
@@ -310,7 +337,9 @@ exports[`Canvas event data HTML event "dragover" should fire a corresponding can
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "viewportPoint": Point {
         "x": 50,
@@ -353,7 +382,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "viewportPoint": Point {
         "x": 50,
@@ -382,7 +413,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "viewportPoint": Point {
         "x": 50,
@@ -411,7 +444,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas
         "x": -30,
         "y": -13,
       },
-      "subTargets": [],
+      "subTargets": [
+        "Drag Target",
+      ],
       "target": "Drag Target",
       "viewportPoint": Point {
         "x": 50,