Bug 1061996 - [Camera] Switching between the Camera and Video buttons…

… rapidly can disable them

apps/camera/js/controllers/camera.js

@@ -218,7 +218,10 @@ CameraController.prototype.setMode = function(mode) {
   // if the value is an object. Quite a risky change
   // to make, but would remove the need for us to check
   // here and in other change callbacks. Food 4 thought :)
-  if (this.camera.isMode(mode)) { return; }
+  if (this.camera.isMode(mode)) {
+    debug('mode didn\'t change');
+    return;
+  }
 
   this.setFlashMode();
   this.app.emit('camera:willchange');

apps/camera/js/controllers/controls.js

@@ -241,20 +241,20 @@ ControlsController.prototype.captureHighlightOff = function() {
 };
 
 /**
- * Switch to the next capture
- * mode: 'picture' or 'video'.
+ * Switch to the next capture mode:
+ * 'picture' or 'video', when the
+ * mode is changed via the view.
+ *
+ * Them mode can be changed by either
+ * tapping or swiping the mode switch.
  *
  * @private
  */
-ControlsController.prototype.onViewModeChanged = function(mode) {
-  debug('view mode changed mode: %s', mode);
-  var setting = this.app.settings.mode;
-  this.view.disable();
-  if (mode) { setting.select(mode); }
-  else { setting.next(); }
+ControlsController.prototype.onViewModeChanged = function() {
+  debug('view mode changed');
+  this.app.settings.mode.next();
 };
 
-
 ControlsController.prototype.onCancelButtonClick = function() {
   this.app.emit('activitycanceled');
 };

apps/camera/js/controllers/settings.js

@@ -198,7 +198,7 @@ SettingsController.prototype.closeSettings = function(done) {
     self.view = null;
     self.app.emit('settings:closed');
     debug('settings closed');
-    if (done) { done(); }
+    if (typeof done === 'function') { done(); }
   });
 };
 

apps/camera/js/views/controls.js

@@ -6,6 +6,7 @@ define(function(require, exports, module) {
  */
 
 var debug = require('debug')('view:controls');
+var debounce = require('lib/debounce');
 var bind = require('lib/bind');
 var View = require('view');
 var Drag = require('drag');
@@ -18,7 +19,8 @@ module.exports = View.extend({
   name: 'controls',
   className: 'test-controls',
 
-  initialize: function() {
+  initialize: function(options) {
+    this.drag = options && options.drag; // test hook
     this.render();
   },
 
@@ -50,42 +52,65 @@ module.exports = View.extend({
     return this.bindEvents();
   },
 
+  /**
+   * Respond to click events on the buttons
+   * other than the switch, which is a special
+   * case.
+   *
+   * We 'debouce' the callback to defend
+   * against button-bashing.
+   *
+   * @return {ControlsView} for chaining
+   * @private
+   */
   bindEvents: function() {
+    this.onButtonClick = debounce(this.onButtonClick, 300, true);
     bind(this.els.thumbnail, 'click', this.onButtonClick);
     bind(this.els.capture, 'click', this.onButtonClick);
     bind(this.els.cancel, 'click', this.onButtonClick);
     return this;
   },
 
+  /**
+   * Create the draggable switch.
+   *
+   * We debouce the tapped callback to
+   * defend against button-bashing.
+   *
+   * @private
+   */
   setupSwitch: function() {
     debug('setup dragger');
-    this.drag = new Drag({
+
+    // Prefer existing drag (test hook)
+    this.drag = this.drag || new Drag({
       handle: this.els.switchHandle,
       container: this.els.switch,
     });
 
+    this.drag.on('tapped', debounce(this.onSwitchTapped, 300, true));
     this.drag.on('ended', this.drag.snapToClosestEdge);
     this.drag.on('translate', this.onSwitchTranslate);
     this.drag.on('snapped', this.onSwitchSnapped);
-    this.drag.on('tapped', this.onSwitchTapped);
+
     this.drag.updateDimensions();
     this.updateSwitchPosition();
   },
 
   onSwitchSnapped: function(edges) {
     var mode = this.switchPositions[edges.x];
     var changed = mode !== this.get('mode');
-    if (changed) { this.onSwitchChanged(mode); }
+    if (changed) { this.onSwitchChanged(); }
   },
 
-  onSwitchChanged: function(mode) {
-    this.emit('modechanged', mode);
+  onSwitchChanged: function() {
+    this.emit('modechanged');
   },
 
   onSwitchTapped: function(e) {
     e.preventDefault();
     debug('switch tapped');
-    this.emit('modechanged');
+    this.onSwitchChanged();
   },
 
   onSwitchTranslate: function(e) {

apps/camera/test/unit/controllers/controls_test.js

@@ -39,6 +39,7 @@ suite('controllers/controls', function() {
     // Aliases
     this.controls = this.app.views.controls;
     this.view = this.app.views.controls;
+    this.settings = this.app.settings;
 
     this.controller = new this.ControlsController(this.app);
     this.state = {};
@@ -149,4 +150,11 @@ suite('controllers/controls', function() {
       assert.isTrue(this.app.emit.calledWith('capture'));
     });
   });
+
+  suite('ControlsController.onViewModeChanged()', function() {
+    test('It switches to the next mode setting', function() {
+      this.controller.onViewModeChanged();
+      sinon.assert.called(this.settings.mode.next);
+    });
+  });
 });

apps/camera/test/unit/views/controls_test.js

@@ -1,11 +1,15 @@
-suite('views/preview-gallery', function() {
+suite('views/controls', function() {
   /*jshint maxlen:false*/
   'use strict';
 
   suiteSetup(function(done) {
     var self = this;
-    requirejs(['views/controls'], function(ControlsView) {
+    requirejs([
+      'views/controls',
+      'drag'
+    ], function(ControlsView, Drag) {
       self.ControlsView = ControlsView;
+      self.Drag = Drag;
       done();
     });
   });
@@ -28,8 +32,10 @@ suite('views/preview-gallery', function() {
     this.sandbox.stub(window.URL, 'revokeObjectURL');
 
     this.sandbox.stub(window, 'Image', function() { return self.image; });
+    this.sandbox.spy(this.ControlsView.prototype, 'onSwitchTapped');
 
-    this.view = new this.ControlsView();
+    this.drag = sinon.createStubInstance(this.Drag);
+    this.view = new this.ControlsView({ drag: this.drag });
     this.view.els.image = undefined;
     this.classes = this.view.el.classList;
 
@@ -157,9 +163,8 @@ suite('views/preview-gallery', function() {
 
   suite('ControlsView#onSwitchTapped()', function() {
     setup(function() {
-      this.event = {
-        preventDefault: sinon.spy()
-      };
+      this.event = { preventDefault: sinon.spy() };
+      this.spy = this.ControlsView.prototype.onSwitchTapped;
     });
 
     test('It emits a `modechanged` event', function() {
@@ -171,5 +176,34 @@ suite('views/preview-gallery', function() {
       this.view.onSwitchTapped(this.event);
       sinon.assert.called(this.event.preventDefault);
     });
+
+    test('It is debounced to defend against button bashing', function() {
+      this.view.setupSwitch();
+      var callback = this.drag.on.withArgs('tapped').args[0][1];
+
+      callback(this.event);
+      callback(this.event);
+      callback(this.event);
+      callback(this.event);
+
+      sinon.assert.calledOnce(this.spy);
+    });
+  });
+
+  suite('ControlsView#onSwitchSnapped()', function() {
+    setup(function() {
+      this.view.set('mode', 'picture');
+    });
+
+    test('It fires \'modechanged\' when the switch changes position', function() {
+
+      // Didn't change
+      this.view.onSwitchSnapped({ x: 'left' });
+      assert.isFalse(this.view.emit.calledWith('modechanged'));
+
+      // Changed
+      this.view.onSwitchSnapped({ x: 'right' });
+      assert.isTrue(this.view.emit.calledWith('modechanged'));
+    });
   });
 });