From 81c7b778545a945f138c7ee36da0cb3b223c8767 Mon Sep 17 00:00:00 2001
From: Andrea Bogazzi <andreabogazzi79@gmail.com>
Date: Sun, 28 Oct 2018 07:49:59 +0100
Subject: [PATCH 1/3] add onSelect on the grouping logic

---
 src/mixins/canvas_grouping.mixin.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js
index 014bec88b3c..86c8b50b7bd 100644
--- a/src/mixins/canvas_grouping.mixin.js
+++ b/src/mixins/canvas_grouping.mixin.js
@@ -14,7 +14,7 @@
     _shouldGroup: function(e, target) {
       var activeObject = this._activeObject;
       return activeObject && this._isSelectionKeyPressed(e) && target && target.selectable && this.selection &&
-            (activeObject !== target || activeObject.type === 'activeSelection');
+            (activeObject !== target || activeObject.type === 'activeSelection') && !target.onSelect({ e: e });
     },
 
     /**
@@ -96,7 +96,7 @@
      */
     _groupSelectedObjects: function (e) {
 
-      var group = this._collectObjects(),
+      var group = this._collectObjects(e),
           aGroup;
 
       // do not create group for 1 element only
@@ -114,7 +114,7 @@
     /**
      * @private
      */
-    _collectObjects: function() {
+    _collectObjects: function(e) {
       var group = [],
           currentObject,
           x1 = this._groupSelector.ex,
@@ -129,7 +129,7 @@
       for (var i = this._objects.length; i--; ) {
         currentObject = this._objects[i];
 
-        if (!currentObject || !currentObject.selectable || !currentObject.visible) {
+        if (!currentObject || !currentObject.selectable || !currentObject.visible || currentObject.onSelect({ e: e })) {
           continue;
         }
 

From 053a711305a2a4e6b734720facbdd5b682430662 Mon Sep 17 00:00:00 2001
From: Andrea Bogazzi <andreabogazzi79@gmail.com>
Date: Sun, 28 Oct 2018 08:47:22 +0100
Subject: [PATCH 2/3] added test for group selector

---
 test/unit/canvas.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/test/unit/canvas.js b/test/unit/canvas.js
index 94c1e7885a2..2a05fda67a2 100644
--- a/test/unit/canvas.js
+++ b/test/unit/canvas.js
@@ -573,6 +573,25 @@
     canvas.selectionFullyContained = false;
   });
 
+  QUnit.test('_collectObjects does not collect objects that have onSelect returning true', function(assert) {
+    canvas.selectionFullyContained = false;
+    var rect1 = new fabric.Rect({ width: 10, height: 10, top: 2, left: 2 });
+    rect1.onSelect = function() {
+      return true;
+    };
+    var rect2 = new fabric.Rect({ width: 10, height: 10, top: 2, left: 2 });
+    canvas.add(rect1, rect2);
+    canvas._groupSelector = {
+      top: 20,
+      left: 20,
+      ex: 1,
+      ey: 1
+    };
+    var collected = canvas._collectObjects();
+    assert.equal(collected.length, 1, 'objects are in the same position buy only one gets selected');
+    assert.equal(collected[0], rect2, 'contains rect2 but not rect 1');
+  });
+
   QUnit.test('_fireSelectionEvents fires multiple things', function(assert) {
     var rect1Deselected = false;
     var rect3Selected = false;

From 1a5da77a0c9388ae42667b3fdef24e7367965371 Mon Sep 17 00:00:00 2001
From: Andrea Bogazzi <andreabogazzi79@gmail.com>
Date: Sun, 28 Oct 2018 09:40:53 +0100
Subject: [PATCH 3/3] another test

---
 src/mixins/canvas_grouping.mixin.js |  1 +
 test/unit/canvas.js                 | 42 +++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js
index 86c8b50b7bd..373f288461f 100644
--- a/src/mixins/canvas_grouping.mixin.js
+++ b/src/mixins/canvas_grouping.mixin.js
@@ -24,6 +24,7 @@
      */
     _handleGrouping: function (e, target) {
       var activeObject = this._activeObject;
+      // avoid multi select when shift click on a corner
       if (activeObject.__corner) {
         return;
       }
diff --git a/test/unit/canvas.js b/test/unit/canvas.js
index 2a05fda67a2..e8a74dc7c09 100644
--- a/test/unit/canvas.js
+++ b/test/unit/canvas.js
@@ -592,6 +592,48 @@
     assert.equal(collected[0], rect2, 'contains rect2 but not rect 1');
   });
 
+  QUnit.test('_shouldGroup return false if onSelect return true', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return true;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = true;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, false, 'if onSelect returns true, shouldGroup return false');
+  });
+
+  QUnit.test('_shouldGroup return true if onSelect return false and selectionKey is true', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return false;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = true;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, true, 'if onSelect returns false, shouldGroup return true');
+  });
+
+  QUnit.test('_shouldGroup return false if selectionKey is false', function(assert) {
+    var rect = new fabric.Rect();
+    var rect2 = new fabric.Rect();
+    rect.onSelect = function() {
+      return false;
+    };
+    canvas._activeObject = rect2;
+    var selectionKey = canvas.selectionKey;
+    var event = {};
+    event[selectionKey] = false;
+    var returned = canvas._shouldGroup(event, rect);
+    assert.equal(returned, false, 'shouldGroup return false');
+  });
+
   QUnit.test('_fireSelectionEvents fires multiple things', function(assert) {
     var rect1Deselected = false;
     var rect3Selected = false;