From 6d62d118722d199c94b8a995526f3821f28c9c0b Mon Sep 17 00:00:00 2001
From: jdescottes <julian.descottes@gmail.com>
Date: Sun, 30 Mar 2014 20:39:00 +0200
Subject: [PATCH] Fix : drawing with the right mouse button (FF, IE)

Mousemove events do not have the correct button information
Need to keep the state of which button is clicked at which moment

This was actually the initial implementation ...
But I removed it because I couldn't remember why we did this in the
beginning.

Added lots of 'warnings' in comments, but won't be safe until we get good
integration tests.

References :
- FF : https://bugzilla.mozilla.org/show_bug.cgi?id=297919
"What if multiple buttons are pressed during mouse move?  And how does the
integer field (button) indicate that no button is pressed?  I think this
should be wontfix -- if people need button information during mouse move events
we need a new way of getting it (buttonSet), not the same way it works for click,
etc."
- IE : couldn't find any reference ...
---
 src/js/controller/DrawingController.js      | 28 ++++++++++++++++-----
 src/js/controller/PalettesListController.js |  7 ++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/src/js/controller/DrawingController.js b/src/js/controller/DrawingController.js
index ee7ab623..b5d974ae 100644
--- a/src/js/controller/DrawingController.js
+++ b/src/js/controller/DrawingController.js
@@ -42,6 +42,9 @@
     this.isClicked = false;
     this.previousMousemoveTime = 0;
     this.currentToolBehavior = null;
+
+    // State of clicked button (need to be stateful here, see comment in getCurrentColor_)
+    this.currentMouseButton_ = Constants.LEFT_BUTTON;
   };
 
   ns.DrawingController.prototype.init = function () {
@@ -140,12 +143,13 @@
       }
     } else {
       this.isClicked = true;
+      this.setCurrentButton(event);
       this.currentToolBehavior.hideHighlightedPixel(this.overlayFrame);
 
       this.currentToolBehavior.applyToolAt(
         coords.x,
         coords.y,
-        this.getCurrentColor_(event),
+        this.getCurrentColor_(),
         frame,
         this.overlayFrame,
         event
@@ -166,7 +170,8 @@
       var coords = this.renderer.getCoordinates(event.clientX, event.clientY);
 
       if (this.isClicked) {
-
+        // Warning : do not call setCurrentButton here
+        // mousemove do not have the correct mouse button information on all browsers
         this.currentToolBehavior.moveToolAt(
           coords.x,
           coords.y,
@@ -222,12 +227,13 @@
       // of the drawing canvas.
 
       this.isClicked = false;
+      this.setCurrentButton(event);
 
       var coords = this.renderer.getCoordinates(event.clientX, event.clientY);
       this.currentToolBehavior.releaseToolAt(
         coords.x,
         coords.y,
-        this.getCurrentColor_(event),
+        this.getCurrentColor_(),
         this.piskelController.getCurrentFrame(),
         this.overlayFrame,
         event
@@ -244,13 +250,23 @@
     return this.renderer.getCoordinates(event.clientX, event.clientY);
   };
 
+  ns.DrawingController.prototype.setCurrentButton = function (event) {
+    this.currentMouseButton_ = event.button;
+  };
+
   /**
    * @private
    */
-  ns.DrawingController.prototype.getCurrentColor_ = function (event) {
-    if(event.button == Constants.RIGHT_BUTTON) {
+  ns.DrawingController.prototype.getCurrentColor_ = function () {
+    // WARNING : Do not rely on the current event to get the current color!
+    // It might seem like a good idea, and works perfectly fine on Chrome
+    // Sadly Firefox and IE found clever, for some reason, to set event.button to 0
+    // on a mouse move event
+    // This always matches a LEFT mouse button which is __really__ not helpful
+
+    if(this.currentMouseButton_ == Constants.RIGHT_BUTTON) {
       return this.paletteController.getSecondaryColor();
-    } else if(event.button == Constants.LEFT_BUTTON) {
+    } else if(this.currentMouseButton_ == Constants.LEFT_BUTTON) {
       return this.paletteController.getPrimaryColor();
     } else {
       return Constants.DEFAULT_PEN_COLOR;
diff --git a/src/js/controller/PalettesListController.js b/src/js/controller/PalettesListController.js
index c2ffb3c4..bc6a445f 100644
--- a/src/js/controller/PalettesListController.js
+++ b/src/js/controller/PalettesListController.js
@@ -98,7 +98,14 @@
 
   ns.PalettesListController.prototype.onColorUpdated = function (type, event, color) {
     console.log('[PalettesListController] >>> ', arguments);
+
     var colorContainer = this.colorListContainer_.querySelector('.palettes-list-color[data-color="'+color+'"]');
+
+    // Color is not in the currently selected palette
+    if (!colorContainer) {
+      return;
+    }
+
     if (type === 'primary') {
       this.removeClass_('primary', '.palettes-list-color');
       colorContainer.classList.add('primary');