Bug 1003761 - Fix clicking on shortcuts in GCLI output. r=robcee

toolkit/devtools/gcli/source/lib/gcli/cli.js

@@ -492,6 +492,10 @@ function Requisition(options) {
 
   addMapping(this);
   this._setBlankAssignment(this.commandAssignment);
+
+  // If a command calls context.update then the UI needs some way to be
+  // informed of the change
+  this.onExternalUpdate = util.createEvent('Requisition.onExternalUpdate');
 }
 
 /**
@@ -584,8 +588,8 @@ Object.defineProperty(Requisition.prototype, 'executionContext', {
       if (legacy) {
         this._executionContext.createView = view.createView;
         this._executionContext.exec = this.exec.bind(this);
-        this._executionContext.update = this.update.bind(this);
+        this._executionContext.update = this._contextUpdate.bind(this);
-        this._executionContext.updateExec = this.updateExec.bind(this);
+        this._executionContext.updateExec = this._contextUpdateExec.bind(this);
 
         Object.defineProperty(this._executionContext, 'document', {
           get: function() { return requisition.document; },
@@ -612,8 +616,8 @@ Object.defineProperty(Requisition.prototype, 'conversionContext', {
 
         createView: view.createView,
         exec: this.exec.bind(this),
-        update: this.update.bind(this),
+        update: this._contextUpdate.bind(this),
-        updateExec: this.updateExec.bind(this)
+        updateExec: this._contextUpdateExec.bind(this)
       };
 
       // Alias requisition so we're clear about what's what
@@ -766,17 +770,36 @@ Requisition.prototype._getFirstBlankPositionalAssignment = function() {
   return reply;
 };
 
+/**
+ * The update process is asynchronous, so there is (unavoidably) a window
+ * where we've worked out the command but don't yet understand all the params.
+ * If we try to do things to a requisition in this window we may get
+ * inconsistent results. Asynchronous promises have made the window bigger.
+ * The only time we've seen this in practice is during focus events due to
+ * clicking on a shortcut. The focus want to check the cursor position while
+ * the shortcut is updating the command line.
+ * This function allows us to detect and back out of this problem.
+ * We should be able to remove this function when all the state in a
+ * requisition can be encapsulated and updated atomically.
+ */
+Requisition.prototype.isUpToDate = function() {
+  if (!this._args) {
+    return false;
+  }
+  for (var i = 0; i < this._args.length; i++) {
+    if (this._args[i].assignment == null) {
+      return false;
+    }
+  }
+  return true;
+};
+
 /**
  * Look through the arguments attached to our assignments for the assignment
  * at the given position.
  * @param {number} cursor The cursor position to query
  */
 Requisition.prototype.getAssignmentAt = function(cursor) {
-  if (!this._args) {
-    console.trace();
-    throw new Error('Missing args');
-  }
-
   // We short circuit this one because we may have no args, or no args with
   // any size and the alg below only finds arguments with size.
   if (cursor === 0) {
@@ -822,14 +845,7 @@ Requisition.prototype.getAssignmentAt = function(cursor) {
   // Possible shortcut, we don't really need to go through all the args
   // to work out the solution to this
 
-  var reply = assignForPos[cursor - 1];
+  return assignForPos[cursor - 1];
-
-  if (!reply) {
-    throw new Error('Missing assignment.' +
-        ' cursor=' + cursor + ' text=' + this.toString());
-  }
-
-  return reply;
 };
 
 /**
@@ -1478,15 +1494,31 @@ function getDataCommandAttribute(element) {
   return command;
 }
 
+/**
+ * Designed to be called from context.update(). Acts just like update() except
+ * that it also calls onExternalUpdate() to inform the UI of an unexpected
+ * change to the current command.
+ */
+Requisition.prototype._contextUpdate = function(typed) {
+  return this.update(typed).then(function(reply) {
+    this.onExternalUpdate({ typed: typed });
+    return reply;
+  }.bind(this));
+};
+
 /**
  * Called by the UI when ever the user interacts with a command line input
- * @param typed The contents of the input field
+ * @param typed The contents of the input field OR an HTML element (or an event
+ * that targets an HTML element) which has a data-command attribute or a child
+ * with the same that contains the command to update with
  */
 Requisition.prototype.update = function(typed) {
-  if (typeof HTMLElement !== 'undefined' && typed instanceof HTMLElement) {
+  // Should be "if (typed instanceof HTMLElement)" except Gecko
+  if (typeof typed.querySelector === 'function') {
     typed = getDataCommandAttribute(typed);
   }
-  if (typeof Event !== 'undefined' && typed instanceof Event) {
+  // Should be "if (typed instanceof Event)" except Gecko
+  if (typeof typed.currentTarget === 'object') {
     typed = getDataCommandAttribute(typed.currentTarget);
   }
 
@@ -2068,6 +2100,18 @@ Requisition.prototype.exec = function(options) {
   }
 };
 
+/**
+ * Designed to be called from context.updateExec(). Acts just like updateExec()
+ * except that it also calls onExternalUpdate() to inform the UI of an
+ * unexpected change to the current command.
+ */
+Requisition.prototype._contextUpdateExec = function(typed, options) {
+  return this.updateExec(typed, options).then(function(reply) {
+    this.onExternalUpdate({ typed: typed });
+    return reply;
+  }.bind(this));
+};
+
 /**
  * A shortcut for calling update, resolving the promise and then exec.
  * @param input The string to execute

toolkit/devtools/gcli/source/lib/gcli/languages/command.js

@@ -106,6 +106,8 @@ var commandLanguage = exports.commandLanguage = {
       var mapping = cli.getMapping(this.requisition.executionContext);
       mapping.terminal = this.terminal;
 
+      this.requisition.onExternalUpdate.add(this.textChanged, this);
+
       return this;
     }.bind(this));
   },
@@ -115,6 +117,7 @@ var commandLanguage = exports.commandLanguage = {
     delete mapping.terminal;
 
     this.requisition.commandOutputManager.onOutput.remove(this.outputted, this);
+    this.requisition.onExternalUpdate.remove(this.textChanged, this);
 
     this.terminal = undefined;
     this.requisition = undefined;
@@ -163,7 +166,14 @@ var commandLanguage = exports.commandLanguage = {
   // Called internally whenever we think that the current assignment might
   // have changed, typically on mouse-clicks or key presses.
   caretMoved: function(start) {
+    if (!this.requisition.isUpToDate()) {
+      return;
+    }
     var newAssignment = this.requisition.getAssignmentAt(start);
+    if (newAssignment == null) {
+      return;
+    }
+
     if (this.assignment !== newAssignment) {
       if (this.assignment.param.type.onLeave) {
         this.assignment.param.type.onLeave(this.assignment);

toolkit/devtools/gcli/source/lib/gcli/mozui/inputter.js

@@ -87,6 +87,7 @@ function Inputter(options, components) {
   this.onResize = util.createEvent('Inputter.onResize');
   this.onWindowResize = this.onWindowResize.bind(this);
   this.document.defaultView.addEventListener('resize', this.onWindowResize, false);
+  this.requisition.onExternalUpdate.add(this.textChanged, this);
 
   this._previousValue = undefined;
   this.requisition.update(this.element.value || '');
@@ -99,6 +100,7 @@ Inputter.prototype.destroy = function() {
   this.document.defaultView.removeEventListener('resize', this.onWindowResize, false);
 
   this.requisition.commandOutputManager.onOutput.remove(this.outputted, this);
+  this.requisition.onExternalUpdate.remove(this.textChanged, this);
   if (this.focusManager) {
     this.focusManager.removeMonitoredElement(this.element, 'input');
   }
@@ -309,7 +311,13 @@ Inputter.prototype._checkAssignment = function(start) {
   if (start == null) {
     start = this.element.selectionStart;
   }
+  if (!this.requisition.isUpToDate()) {
+    return;
+  }
   var newAssignment = this.requisition.getAssignmentAt(start);
+  if (newAssignment == null) {
+    return;
+  }
   if (this.assignment !== newAssignment) {
     if (this.assignment.param.type.onLeave) {
       this.assignment.param.type.onLeave(this.assignment);