Bug 930680 - allow inserting ";" in values in style inspector; r=pbrosset

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;

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();

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");
+}

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]

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);