From beb546cc3ea94424c4750f994649e8b77d9962a3 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 13 May 2015 11:37:18 -0700 Subject: [PATCH] Bug 930680 - allow inserting ";" in values in style inspector; r=pbrosset --- browser/devtools/shared/inplace-editor.js | 28 ++++++++---- .../shared/test/browser_inplace-editor-01.js | 32 ++++++++++++++ .../shared/test/unit/test_advanceValidate.js | 33 ++++++++++++++ .../devtools/shared/test/unit/xpcshell.ini | 1 + browser/devtools/styleinspector/rule-view.js | 44 ++++++++++++++++++- 5 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 browser/devtools/shared/test/unit/test_advanceValidate.js diff --git a/browser/devtools/shared/inplace-editor.js b/browser/devtools/shared/inplace-editor.js index 56a061dcf633..034b32e18d19 100644 --- a/browser/devtools/shared/inplace-editor.js +++ b/browser/devtools/shared/inplace-editor.js @@ -69,9 +69,15 @@ Cu.import("resource://gre/modules/devtools/event-emitter.js"); * This function is called before the editor has been torn down. * {function} destroy: * Called when the editor is destroyed and has been torn down. - * {string} advanceChars: - * If any characters in advanceChars are typed, focus will advance - * to the next element. + * {object} advanceChars: + * This can be either a string or a function. + * If it is a string, then if any characters in it are typed, + * focus will advance to the next element. + * Otherwise, if it is a function, then the function will + * be called with three arguments: a key code, the current text, + * and the insertion point. If the function returns true, + * then the focus advance takes place. If it returns false, + * then the character is inserted instead. * {boolean} stopOnReturn: * If true, the return key will not advance the editor to the next * focusable element. @@ -212,10 +218,15 @@ function InplaceEditor(aOptions, aEvent) // Pull out character codes for advanceChars, listing the // characters that should trigger a blur. - this._advanceCharCodes = {}; - let advanceChars = aOptions.advanceChars || ''; - for (let i = 0; i < advanceChars.length; i++) { - this._advanceCharCodes[advanceChars.charCodeAt(i)] = true; + if (typeof(aOptions.advanceChars) === "function") { + this._advanceChars = aOptions.advanceChars; + } else { + let advanceCharcodes = {}; + let advanceChars = aOptions.advanceChars || ''; + for (let i = 0; i < advanceChars.length; i++) { + advanceCharcodes[advanceChars.charCodeAt(i)] = true; + } + this._advanceChars = aCharCode => aCharCode in advanceCharcodes; } // Hide the provided element and add our editor. @@ -931,7 +942,8 @@ InplaceEditor.prototype = { aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_RETURN && aEvent.shiftKey) { prevent = false; - } else if (aEvent.charCode in this._advanceCharCodes + } else if (this._advanceChars(aEvent.charCode, this.input.value, + this.input.selectionStart) || aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_RETURN || aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_TAB) { prevent = true; diff --git a/browser/devtools/shared/test/browser_inplace-editor-01.js b/browser/devtools/shared/test/browser_inplace-editor-01.js index 8b781601b64f..99e22f6b898f 100644 --- a/browser/devtools/shared/test/browser_inplace-editor-01.js +++ b/browser/devtools/shared/test/browser_inplace-editor-01.js @@ -17,6 +17,7 @@ add_task(function*() { yield testReturnCommit(doc); yield testBlurCommit(doc); yield testAdvanceCharCommit(doc); + yield testAdvanceCharsFunction(doc); host.destroy(); gBrowser.removeCurrentTab(); @@ -91,6 +92,37 @@ function testAdvanceCharCommit(doc) { return def.promise; } +function testAdvanceCharsFunction(doc) { + info("Testing advanceChars as a function"); + let def = promise.defer(); + + let firstTime = true; + + createInplaceEditorAndClick({ + initial: "", + advanceChars: function(aCharCode, aText, aInsertionPoint) { + if (aCharCode !== Components.interfaces.nsIDOMKeyEvent.DOM_VK_COLON) { + return false; + } + if (firstTime) { + firstTime = false; + return false; + } + + // Just to make sure we check it somehow. + return aText.length > 0; + }, + start: function(editor) { + for each (let ch in ":Test:") { + EventUtils.sendChar(ch); + } + }, + done: onDone(":Test", true, def) + }, doc); + + return def.promise; +} + function testEscapeCancel(doc) { info("Testing that escape cancels the new value"); let def = promise.defer(); diff --git a/browser/devtools/shared/test/unit/test_advanceValidate.js b/browser/devtools/shared/test/unit/test_advanceValidate.js new file mode 100644 index 000000000000..87194376aacf --- /dev/null +++ b/browser/devtools/shared/test/unit/test_advanceValidate.js @@ -0,0 +1,33 @@ +/* -*- Mode: Javascript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Tests the advanceValidate function from rule-view.js. + +const Cu = Components.utils; +const Ci = Components.interfaces; +let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); +let require = devtools.require; +let {_advanceValidate} = require("devtools/styleinspector/rule-view"); + +// 1 2 3 +// 0123456789012345678901234567890 +let sampleInput = '\\symbol "string" url(somewhere)'; + +function testInsertion(where, result, testName) { + do_print(testName); + equal(_advanceValidate(Ci.nsIDOMKeyEvent.DOM_VK_SEMICOLON, sampleInput, where), + result, "testing _advanceValidate at " + where); +} + +function run_test() { + testInsertion(4, true, "inside a symbol"); + testInsertion(1, false, "after a backslash"); + testInsertion(8, true, "after whitespace"); + testInsertion(11, false, "inside a string"); + testInsertion(24, false, "inside a URL"); + testInsertion(31, true, "at the end"); +} diff --git a/browser/devtools/shared/test/unit/xpcshell.ini b/browser/devtools/shared/test/unit/xpcshell.ini index 35f8aa177302..71bad0b6bd75 100644 --- a/browser/devtools/shared/test/unit/xpcshell.ini +++ b/browser/devtools/shared/test/unit/xpcshell.ini @@ -5,6 +5,7 @@ tail = firefox-appdir = browser skip-if = toolkit == 'android' || toolkit == 'gonk' +[test_advanceValidate.js] [test_attribute-parsing-01.js] [test_attribute-parsing-02.js] [test_bezierCanvas.js] diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index a20c93a59855..1e26110f121b 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -2796,7 +2796,7 @@ TextPropertyEditor.prototype = { done: this._onValueDone, destroy: this.update, validate: this._onValidate, - advanceChars: ';', + advanceChars: advanceValidate, contentType: InplaceEditor.CONTENT_TYPES.CSS_VALUE, property: this.prop, popup: this.popup @@ -3525,6 +3525,48 @@ function getPropertyNameAndValue(node) { } } +/** + * Called when a character is typed in a value editor. This decides + * whether to advance or not, first by checking to see if ";" was + * typed, and then by lexing the input and seeing whether the ";" + * would be a terminator at this point. + * + * @param {number} aKeyCode Key code to be checked. + * @param {String} aValue Current text editor value. + * @param {number} aInsertionPoint The index of the insertion point. + * @return {Boolean} True if the focus should advance; false if + * the character should be inserted. + */ +function advanceValidate(aKeyCode, aValue, aInsertionPoint) { + // Only ";" has special handling here. + if (aKeyCode !== Ci.nsIDOMKeyEvent.DOM_VK_SEMICOLON) { + return false; + } + + // Insert the character provisionally and see what happens. If we + // end up with a ";" symbol token, then the semicolon terminates the + // value. Otherwise it's been inserted in some spot where it has a + // valid meaning, like a comment or string. + aValue = aValue.slice(0, aInsertionPoint) + ';' + aValue.slice(aInsertionPoint); + let lexer = domUtils.getCSSLexer(aValue); + while (true) { + let token = lexer.nextToken(); + if (token.endOffset > aInsertionPoint) { + if (token.tokenType === "symbol" && token.text === ";") { + // The ";" is a terminator. + return true; + } else { + // The ";" is not a terminator in this context. + break; + } + } + } + return false; +} + +// We're exporting _advanceValidate for unit tests. +exports._advanceValidate = advanceValidate; + XPCOMUtils.defineLazyGetter(this, "clipboardHelper", function() { return Cc["@mozilla.org/widget/clipboardhelper;1"]. getService(Ci.nsIClipboardHelper);