[Bug]: Issues with simultaneous pressing of left and middle mouse buttons

MikeEngler1985 commented 5 months ago

CheckList

[X] I agree to follow this project's Code of Conduct
[X] I have read and followed the Contributing Guide
[X] I have read and followed the Issue Tracker Guide
[X] I have searched and referenced existing issues and discussions
[X] I am filing a BUG report.
[X] I have managed to reproduce the bug after upgrading to the latest version
[X] I have created an accurate and minimal reproduction

Version

5.3.0

In What environments are you experiencing the problem?

Chrome

Node Version (if applicable)

None

Link To Reproduction

http://fabricjs.com/

Steps To Reproduce

Open the link in a web browser.
Click and hold the left mouse button on the rectangle to start dragging it.
While still holding down the left mouse button, press the middle mouse button.
After releasing all mouse buttons, the previously grabbed object always stays at the mouse position, no matter where I move it.

Expected Behavior

The rectangle should not remain 'attached' to the mouse.

Actual Behavior

The rectangle still 'sticks' to the mouse after all buttons have been released.

Error Message & Stack Trace

No Messages

asturur commented 5 months ago

Can you reprhase the point 4 of the reproduction steps? I can understand what you mean with The rectangle should still be 'stuck' to the mouse.

MikeEngler1985 commented 5 months ago

Can you reprhase the point 4 of the reproduction steps? I can understand what you mean with The rectangle should still be 'stuck' to the mouse.

Done

asturur commented 5 months ago

Yeah ok, i think this is because we return if we detect a mouse button that is not the left one, so we don't execute the mouseup logic. I think this is fixable by returning only if the mouse buttons are more than one. I won't have time to work on 5.x because there is still work to finish v6.

I do not know how your code looks like, but i don't have a mouse with a middle button ( i have just the mousewheel, is that the one? ) but i don't have it connected to my dev laptop

  __onMouseUp(e) {
    var _this$_activeObject;
    this._cacheTransformEventData(e);
    this._handleEvent(e, 'up:before');
    const transform = this._currentTransform;
    const isClick = this._isClick;
    const target = this._target;

    // if right/middle click just fire events and return
    // target undefined will make the _handleEvent search the target
    const {
      button
    } = e;
    if (button) {
      (this.fireMiddleClick && button === 1 || this.fireRightClick && button === 2) && this._handleEvent(e, 'up');
      this._resetTransformEventData();
      return;
    }
    if (this.isDrawingMode && this._isCurrentlyDrawing) {
      this._onMouseUpInDrawingMode(e);
      return;
    }
    if (!this._isMainEvent(e)) {
      return;
    }
    // ... deselection code

This is the function responsible for mouse up in 5.x I suspect you have fireMiddleClick set to true?

MikeEngler1985 commented 5 months ago

I do not know how your code looks like, but i don't have a mouse with a middle button ( i have just the mousewheel, is that the one? ) but i don't have it connected to my dev laptop

Thank you very much for the quick response. Yes, I do have a mouse wheel and I can press it additionally. For testing purposes, I have this simple code and here lies the described problem.

<!DOCTYPE html>
<html>

<head>
  <title>FabricJS Example</title>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/fabric.js/5.3.1/fabric.min.js"></script>
  <style>
    #container {
      display: flex;
      justify-content: center;
      align-items: center;
      height: 100vh;
    }

    #c {
      border: 1px solid white;
    }
  </style>
</head>

<body>
  <div id="container">
    <canvas id="c" width="500" height="500"></canvas>
  </div>
  <script>
    const canvas = new fabric.Canvas("c");
    canvas.backgroundColor = "gray";
    let rect = new fabric.Rect({
      left: 200,
      top: 200,
      fill: "transparent",
      width: 100,
      height: 100,
      stroke: "white",
      strokeWidth: 2,
      selectable: true
    });
    canvas.add(rect);

    let isDragging = false;

    canvas.on('mouse:down', function (e) {
      if (e.e.button === 0) {
        isDragging = true;
      }
    });

    canvas.on('mouse:move', function (e) {
      if (e.e.button === 1 && isDragging) {
        canvas.discardActiveObject().renderAll();
        isDragging = false;
      }
    });

    canvas.on('mouse:up', function (e) {
      if (e.e.button === 0) {
        isDragging = false;
      }
    });
  </script>
</body>

</html>

asturur commented 5 months ago

so you are no using either fireMiddleClick or fireRightClick functions?

MikeEngler1985 commented 5 months ago

so you are no using either fireMiddleClick or fireRightClick functions?

In the maincode I use the fireMiddleClick to move the canvas.

asturur commented 5 months ago

so please edit the example code to have fireMiddleClick set as true. Does the bug happen with fireMiddleClick set as false?

MikeEngler1985 commented 5 months ago

Yes it happens.

<!DOCTYPE html>
<html>

<head>
  <title>FabricJS Example</title>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/fabric.js/5.3.1/fabric.min.js"></script>
  <style>
    #container {
      display: flex;
      justify-content: center;
      align-items: center;
      height: 100vh;
    }

    #c {
      border: 1px solid white;
    }
  </style>
</head>

<body>
  <div id="container">
    <canvas id="c" width="500" height="500"></canvas>
  </div>
  <script>
    const canvas = new fabric.Canvas("c");
    canvas.backgroundColor = "gray";
    let rect = new fabric.Rect({
      left: 200,
      top: 200,
      fill: "transparent",
      width: 100,
      height: 100,
      stroke: "white",
      strokeWidth: 2,
      selectable: true
    });
    canvas.add(rect);

    canvas.fireMiddleClick = false;

    let isDragging = false;

    canvas.on('mouse:down', function (e) {
      if (e.button === 0) {
        isDragging = true;
      }
    });
    canvas.on('mouse:move', function (e) {
      if (e.button === 1 && isDragging) {
        canvas.discardActiveObject().renderAll();
        isDragging = false;
      }
    });
    canvas.on('mouse:up', function (e) {
      if (e.button === 0) {
        isDragging = false;
      }
    });
  </script>
</body>

</html>

fabricjs / fabric.js