Bug 977586 - omit quotes in top-level strings logged via console.log(), and omit extra spaces when custom styles (%c) are used. r=past,baku
authorDmitry Sagalovskiy <moz.dmitry@getgrist.com>
Mon, 04 May 2015 17:28:25 +0200
changeset 273916 ecfe83c17b85cfff66546bb4a2514fa6dfc9137e
parent 273820 31c33dc5c4dd0e6170b474db98ab4d4eda117f47
child 273917 47a1d2e677853592bcebe5b0a62d8bee39cd1942
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast, baku
bugs977586
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 977586 - omit quotes in top-level strings logged via console.log(), and omit extra spaces when custom styles (%c) are used. r=past,baku
browser/devtools/webconsole/console-output.js
browser/devtools/webconsole/test/browser_webconsole_console_logging_api.js
browser/devtools/webconsole/test/browser_webconsole_output_01.js
browser/devtools/webconsole/test/browser_webconsole_output_events.js
browser/devtools/webconsole/test/browser_webconsole_output_order.js
browser/devtools/webconsole/test/head.js
dom/base/Console.cpp
--- a/browser/devtools/webconsole/console-output.js
+++ b/browser/devtools/webconsole/console-output.js
@@ -1057,26 +1057,26 @@ Messages.Extended.prototype = Heritage.e
    * Render one piece/element of the message array.
    *
    * @private
    * @param mixed piece
    *        Message element to display - this can be a LongString, ObjectActor,
    *        DOM node or a function to invoke.
    * @return Element
    */
-  _renderBodyPiece: function(piece)
+  _renderBodyPiece: function(piece, options = {})
   {
     if (piece instanceof Ci.nsIDOMNode) {
       return piece;
     }
     if (typeof piece == "function") {
       return piece(this);
     }
 
-    return this._renderValueGrip(piece);
+    return this._renderValueGrip(piece, options);
   },
 
   /**
    * Render a grip that represents a value received from the server. This method
    * picks the appropriate widget to render the value with.
    *
    * @private
    * @param object grip
@@ -1405,41 +1405,44 @@ Messages.ConsoleGeneric.prototype = Heri
     let body = Messages.Simple.prototype._renderBody.apply(this, arguments);
     body.classList.remove("devtools-monospace", "message-body");
     return body;
   },
 
   _renderBodyPieces: function(container)
   {
     let lastStyle = null;
+    let stylePieces = this._styles.length > 0 ? this._styles.length : 1;
 
     for (let i = 0; i < this._messagePieces.length; i++) {
-      let separator = i > 0 ? this._renderBodyPieceSeparator() : null;
-      if (separator) {
-        container.appendChild(separator);
+      // Pieces with an associated style definition come from "%c" formatting.
+      // For body pieces beyond that, add a separator before each one.
+      if (i >= stylePieces) {
+        container.appendChild(this._renderBodyPieceSeparator());
       }
 
       let piece = this._messagePieces[i];
       let style = this._styles[i];
 
       // No long string support.
-      if (style && typeof style == "string" ) {
-        lastStyle = this.cleanupStyle(style);
-      }
+      lastStyle = (style && typeof style == "string") ?
+                  this.cleanupStyle(style) : null;
 
       container.appendChild(this._renderBodyPiece(piece, lastStyle));
     }
 
     this._messagePieces = null;
     this._styles = null;
   },
 
   _renderBodyPiece: function(piece, style)
   {
-    let elem = Messages.Extended.prototype._renderBodyPiece.call(this, piece);
+    // Skip quotes for top-level strings.
+    let options = { noStringQuotes: true };
+    let elem = Messages.Extended.prototype._renderBodyPiece.call(this, piece, options);
     let result = elem;
 
     if (style) {
       if (elem.nodeType == Ci.nsIDOMNode.ELEMENT_NODE) {
         elem.style = style;
       } else {
         let span = this.document.createElementNS(XHTML_NS, "span");
         span.style = style;
@@ -3293,21 +3296,26 @@ Widgets.ObjectRenderers.add({
 /**
  * The long string widget.
  *
  * @constructor
  * @param object message
  *        The owning message.
  * @param object longStringActor
  *        The LongStringActor to display.
+ * @param object options
+ *        Options, such as noStringQuotes
  */
-Widgets.LongString = function(message, longStringActor)
+Widgets.LongString = function(message, longStringActor, options)
 {
   Widgets.BaseWidget.call(this, message);
   this.longStringActor = longStringActor;
+  this.noStringQuotes = (options && "noStringQuotes" in options) ?
+    options.noStringQuotes : !this.message._quoteStrings;
+
   this._onClick = this._onClick.bind(this);
   this._onSubstring = this._onSubstring.bind(this);
 };
 
 Widgets.LongString.prototype = Heritage.extend(Widgets.BaseWidget.prototype,
 {
   /**
    * The LongStringActor displayed by the widget.
@@ -3333,17 +3341,17 @@ Widgets.LongString.prototype = Heritage.
    * Render the long string in the widget element.
    * @private
    * @param string str
    *        The string to display.
    */
   _renderString: function(str)
   {
     this.element.textContent = VariablesView.getString(str, {
-      noStringQuotes: !this.message._quoteStrings,
+      noStringQuotes: this.noStringQuotes,
       noEllipsis: true,
     });
   },
 
   /**
    * Render the anchor ellipsis that allows the user to expand the long string.
    *
    * @private
--- a/browser/devtools/webconsole/test/browser_webconsole_console_logging_api.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_console_logging_api.js
@@ -84,17 +84,17 @@ function* testMethod(aMethod, aHud, aOut
   aHud.jsterm.clearOutput();
 
   // test for multiple arguments.
   console[aMethod]("foo", "bar");
 
   yield waitForMessages({
     webconsole: aHud,
     messages: [{
-      text: '"foo" "bar"',
+      text: 'foo bar',
       category: CATEGORY_WEBDEV,
     }],
   })
 }
 
 function setStringFilter(aValue, aHud) {
   aHud.ui.filterBox.value = aValue;
   aHud.ui.adjustVisibilityOnSearchStringChange();
--- a/browser/devtools/webconsole/test/browser_webconsole_output_01.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_output_01.js
@@ -24,61 +24,67 @@ DebuggerServer.LONG_STRING_INITIAL_LENGT
 let longString = (new Array(DebuggerServer.LONG_STRING_LENGTH + 4)).join("a");
 let initialString = longString.substring(0, DebuggerServer.LONG_STRING_INITIAL_LENGTH);
 
 let inputTests = [
   // 0
   {
     input: "'hello \\nfrom \\rthe \\\"string world!'",
     output: "\"hello \nfrom \rthe \"string world!\"",
+    consoleOutput: "hello \nfrom \rthe \"string world!",
   },
 
   // 1
   {
     // unicode test
     input: "'\xFA\u1E47\u0129\xE7\xF6d\xEA \u021B\u0115\u0219\u0165'",
     output: "\"\xFA\u1E47\u0129\xE7\xF6d\xEA \u021B\u0115\u0219\u0165\"",
+    consoleOutput: "\xFA\u1E47\u0129\xE7\xF6d\xEA \u021B\u0115\u0219\u0165",
   },
 
   // 2
   {
     input: "'" + longString + "'",
     output: '"' + initialString + "\"[\u2026]",
+    consoleOutput: initialString + "[\u2026]",
     printOutput: initialString,
   },
 
   // 3
   {
     input: "''",
     output: '""',
+    consoleOutput: "",
     printOutput: '""',
   },
 
   // 4
   {
     input: "0",
     output: "0",
   },
 
   // 5
   {
     input: "'0'",
     output: '"0"',
+    consoleOutput: "0",
   },
 
   // 6
   {
     input: "42",
     output: "42",
   },
 
   // 7
   {
     input: "'42'",
     output: '"42"',
+    consoleOutput: "42",
   },
 
   // 8
   {
     input: "/foobar/",
     output: "/foobar/",
     inspectable: true,
   },
--- a/browser/devtools/webconsole/test/browser_webconsole_output_events.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_output_events.js
@@ -30,24 +30,24 @@ let test = asyncTest(function* () {
       category: CATEGORY_OUTPUT,
     }],
   });
 
   yield waitForMessages({
     webconsole: hud,
     messages: [{
       name: "console.log() output for mousemove",
-      text: /"eventLogger" mousemove { target: .+, buttons: 0, clientX: \d+, clientY: \d+, layerX: \d+, layerY: \d+ }/,
+      text: /eventLogger mousemove { target: .+, buttons: 0, clientX: \d+, clientY: \d+, layerX: \d+, layerY: \d+ }/,
       category: CATEGORY_WEBDEV,
       severity: SEVERITY_LOG,
     }],
   });
 
   yield waitForMessages({
     webconsole: hud,
     messages: [{
       name: "console.log() output for keypress",
-      text: /"eventLogger" keypress Shift { target: .+, key: .+, charCode: \d+, keyCode: \d+ }/,
+      text: /eventLogger keypress Shift { target: .+, key: .+, charCode: \d+, keyCode: \d+ }/,
       category: CATEGORY_WEBDEV,
       severity: SEVERITY_LOG,
     }],
   });
 });
\ No newline at end of file
--- a/browser/devtools/webconsole/test/browser_webconsole_output_order.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_output_order.js
@@ -26,17 +26,17 @@ let test = asyncTest(function*() {
       text: "console.log('foo', 'bar');",
       category: CATEGORY_INPUT,
     },
     {
       text: "undefined",
       category: CATEGORY_OUTPUT,
     },
     {
-      text: '"foo" "bar"',
+      text: 'foo bar',
       category: CATEGORY_WEBDEV,
       severity: SEVERITY_LOG,
     }],
   });
 
   let fncall_node = [...function_call.matched][0];
   let result_node = [...result.matched][0];
   let console_message_node = [...console_message.matched][0];
--- a/browser/devtools/webconsole/test/head.js
+++ b/browser/devtools/webconsole/test/head.js
@@ -1416,16 +1416,19 @@ function whenDelayedStartupFinished(aWin
  *        - inspectable: boolean, when true, the test runner expects the JS eval
  *        result is an object that can be clicked for inspection.
  *
  *        - noClick: boolean, when true, the test runner does not click the JS
  *        eval result. Some objects, like |window|, have a lot of properties and
  *        opening vview for them is very slow (they can cause timeouts in debug
  *        builds).
  *
+ *        - consoleOutput: string|RegExp, optional, expected consoleOutput
+ *        If not provided consoleOuput = output;
+ *
  *        - printOutput: string|RegExp, optional, expected output for
  *        |print(input)|. If this is not provided, printOutput = output.
  *
  *        - variablesViewLabel: string|RegExp, optional, the expected variables
  *        view label when the object is inspected. If this is not provided, then
  *        |output| is used.
  *
  *        - inspectorIcon: boolean, when true, the test runner expects the
@@ -1457,21 +1460,24 @@ function checkOutputForInputs(hud, input
   }
 
   function* checkConsoleLog(entry)
   {
     info("Logging: " + entry.input);
     hud.jsterm.clearOutput();
     hud.jsterm.execute("console.log(" + entry.input + ")");
 
+    let consoleOutput = "consoleOutput" in entry ?
+                        entry.consoleOutput : entry.output;
+
     let [result] = yield waitForMessages({
       webconsole: hud,
       messages: [{
-        name: "console.log() output: " + entry.output,
-        text: entry.output,
+        name: "console.log() output: " + consoleOutput,
+        text: consoleOutput,
         category: CATEGORY_WEBDEV,
         severity: SEVERITY_LOG,
       }],
     });
 
     if (typeof entry.inspectorIcon == "boolean") {
       let msg = [...result.matched][0];
       info("Checking Inspector Link: " + entry.input);
--- a/dom/base/Console.cpp
+++ b/dom/base/Console.cpp
@@ -1356,16 +1356,37 @@ Console::ProcessCallData(ConsoleCallData
     innerID.AppendInt(aData->mInnerIDNumber);
   }
 
   if (NS_FAILED(mStorage->RecordEvent(innerID, outerID, eventValue))) {
     NS_WARNING("Failed to record a console event.");
   }
 }
 
+namespace {
+
+// Helper method for ProcessArguments. Flushes output, if non-empty, to aSequence.
+void
+FlushOutput(JSContext* aCx, Sequence<JS::Value>& aSequence, nsString &output)
+{
+  if (!output.IsEmpty()) {
+    JS::Rooted<JSString*> str(aCx, JS_NewUCStringCopyN(aCx,
+                                                       output.get(),
+                                                       output.Length()));
+    if (!str) {
+      return;
+    }
+
+    aSequence.AppendElement(JS::StringValue(str));
+    output.Truncate();
+  }
+}
+
+} // anonymous namespace
+
 void
 Console::ProcessArguments(JSContext* aCx,
                           const nsTArray<JS::Heap<JS::Value>>& aData,
                           Sequence<JS::Value>& aSequence,
                           Sequence<JS::Value>& aStyles)
 {
   if (aData.IsEmpty()) {
     return;
@@ -1467,50 +1488,30 @@ Console::ProcessArguments(JSContext* aCx
     char ch = *start;
     tmp.Append(ch);
     ++start;
 
     switch (ch) {
       case 'o':
       case 'O':
       {
-        if (!output.IsEmpty()) {
-          JS::Rooted<JSString*> str(aCx, JS_NewUCStringCopyN(aCx,
-                                                             output.get(),
-                                                             output.Length()));
-          if (!str) {
-            return;
-          }
-
-          aSequence.AppendElement(JS::StringValue(str));
-          output.Truncate();
-        }
+        FlushOutput(aCx, aSequence, output);
 
         JS::Rooted<JS::Value> v(aCx);
         if (index < aData.Length()) {
           v = aData[index++];
         }
 
         aSequence.AppendElement(v);
         break;
       }
 
       case 'c':
       {
-        if (!output.IsEmpty()) {
-          JS::Rooted<JSString*> str(aCx, JS_NewUCStringCopyN(aCx,
-                                                             output.get(),
-                                                             output.Length()));
-          if (!str) {
-            return;
-          }
-
-          aSequence.AppendElement(JS::StringValue(str));
-          output.Truncate();
-        }
+        FlushOutput(aCx, aSequence, output);
 
         if (index < aData.Length()) {
           JS::Rooted<JS::Value> v(aCx, aData[index++]);
           JS::Rooted<JSString*> jsString(aCx, JS::ToString(aCx, v));
           if (!jsString) {
             return;
           }
 
@@ -1574,24 +1575,21 @@ Console::ProcessArguments(JSContext* aCx
         break;
 
       default:
         output.Append(tmp);
         break;
     }
   }
 
-  if (!output.IsEmpty()) {
-    JS::Rooted<JSString*> str(aCx, JS_NewUCStringCopyN(aCx, output.get(),
-                                                       output.Length()));
-    if (!str) {
-      return;
-    }
+  FlushOutput(aCx, aSequence, output);
 
-    aSequence.AppendElement(JS::StringValue(str));
+  // Discard trailing style element if there is no output to apply it to.
+  if (aStyles.Length() > aSequence.Length()) {
+    aStyles.TruncateLength(aSequence.Length());
   }
 
   // The rest of the array, if unused by the format string.
   for (; index < aData.Length(); ++index) {
     aSequence.AppendElement(aData[index]);
   }
 }