From 2354856cf519cb3361058d4808c15aecc6a0df16 Mon Sep 17 00:00:00 2001 From: Joe Walker Date: Fri, 27 Jun 2014 11:44:00 +0200 Subject: [PATCH] Bug 1003761 - Fix clicking on shortcuts in GCLI output. r=robcee --- toolkit/devtools/gcli/source/lib/gcli/cli.js | 84 ++++++++++++++----- .../gcli/source/lib/gcli/languages/command.js | 10 +++ .../gcli/source/lib/gcli/mozui/inputter.js | 8 ++ 3 files changed, 82 insertions(+), 20 deletions(-) diff --git a/toolkit/devtools/gcli/source/lib/gcli/cli.js b/toolkit/devtools/gcli/source/lib/gcli/cli.js index 40cd7c7e0c7..efa940987bc 100644 --- a/toolkit/devtools/gcli/source/lib/gcli/cli.js +++ b/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.updateExec = this.updateExec.bind(this); + this._executionContext.update = this._contextUpdate.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), - updateExec: this.updateExec.bind(this) + update: this._contextUpdate.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]; - - if (!reply) { - throw new Error('Missing assignment.' + - ' cursor=' + cursor + ' text=' + this.toString()); - } - - return reply; + return assignForPos[cursor - 1]; }; /** @@ -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 diff --git a/toolkit/devtools/gcli/source/lib/gcli/languages/command.js b/toolkit/devtools/gcli/source/lib/gcli/languages/command.js index 3e20d1f760f..2c21d0d5d6f 100644 --- a/toolkit/devtools/gcli/source/lib/gcli/languages/command.js +++ b/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); diff --git a/toolkit/devtools/gcli/source/lib/gcli/mozui/inputter.js b/toolkit/devtools/gcli/source/lib/gcli/mozui/inputter.js index fba5ef31cc8..282fc5134ea 100644 --- a/toolkit/devtools/gcli/source/lib/gcli/mozui/inputter.js +++ b/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);