Bug 955820 - console.group doesn't group messages when timestamps are disabled. r=robcee, a=sledru
authorMihai Sucan <mihai.sucan@gmail.com>
Tue, 18 Feb 2014 22:10:41 +0200
changeset 182942 801ada2044a253a78b9de38d8c7ad03ee51e0b12
parent 182941 06c41a48e27a669121b8cbfa9b170404606462bc
child 182943 57a7218fd75c08ff6f90c62f4cbd92c1f2c9fe94
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobcee, sledru
bugs955820
milestone29.0a2
Bug 955820 - console.group doesn't group messages when timestamps are disabled. r=robcee, a=sledru
browser/devtools/webconsole/console-output.js
browser/devtools/webconsole/test/browser_webconsole_bug_664131_console_group.js
browser/devtools/webconsole/test/head.js
browser/devtools/webconsole/webconsole.js
--- a/browser/devtools/webconsole/console-output.js
+++ b/browser/devtools/webconsole/console-output.js
@@ -60,16 +60,19 @@ const COMPAT = {
   CATEGORY_CLASS_FRAGMENTS: [ "network", "cssparser", "exception", "console",
                               "input", "output", "security" ],
 
   // The fragment of a CSS class name that identifies each severity.
   SEVERITY_CLASS_FRAGMENTS: [ "error", "warn", "info", "log" ],
 
   // The indent of a console group in pixels.
   GROUP_INDENT: 12,
+
+  // The default indent in pixels, applied even without any groups.
+  GROUP_INDENT_DEFAULT: 6,
 };
 
 // A map from the console API call levels to the Web Console severities.
 const CONSOLE_API_LEVELS_TO_SEVERITIES = {
   error: "error",
   exception: "error",
   assert: "error",
   warn: "warning",
@@ -770,16 +773,22 @@ Messages.Simple.prototype = Heritage.ext
       return this;
     }
 
     let timestamp = new Widgets.MessageTimestamp(this, this.timestamp).render();
 
     let icon = this.document.createElementNS(XHTML_NS, "span");
     icon.className = "icon";
 
+    // Apply the current group by indenting appropriately.
+    // TODO: remove this once bug 778766 is fixed.
+    let iconMarginLeft = this._groupDepthCompat * COMPAT.GROUP_INDENT +
+                         COMPAT.GROUP_INDENT_DEFAULT;
+    icon.style.marginLeft = iconMarginLeft + "px";
+
     let body = this._renderBody();
     this._repeatID.textContent += "|" + body.textContent;
 
     let repeatNode = this._renderRepeatNode();
     let location = this._renderLocation();
 
     Messages.BaseMessage.prototype.render.call(this);
     if (this._className) {
@@ -1300,21 +1309,16 @@ Widgets.MessageTimestamp.prototype = Her
     if (this.element) {
       return this;
     }
 
     this.element = this.document.createElementNS(XHTML_NS, "span");
     this.element.className = "timestamp devtools-monospace";
     this.element.textContent = l10n.timestampString(this.timestamp) + " ";
 
-    // Apply the current group by indenting appropriately.
-    // TODO: remove this once bug 778766 is fixed.
-    this.element.style.marginRight = this.message._groupDepthCompat *
-                                     COMPAT.GROUP_INDENT + "px";
-
     return this;
   },
 }); // Widgets.MessageTimestamp.prototype
 
 
 /**
  * The JavaScript object widget.
  *
--- a/browser/devtools/webconsole/test/browser_webconsole_bug_664131_console_group.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_bug_664131_console_group.js
@@ -1,107 +1,79 @@
 /* vim:set ts=2 sw=2 sts=2 et: */
 /*
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 // Tests that console.group/groupEnd works as intended.
-
-let testDriver, hud;
+const TEST_URI = "data:text/html;charset=utf-8,Web Console test for bug 664131: Expand console object with group methods";
 
 function test() {
-  addTab("data:text/html;charset=utf-8,Web Console test for bug 664131: Expand console " +
-         "object with group methods");
-  browser.addEventListener("load", function onLoad(aEvent) {
-    browser.removeEventListener(aEvent.type, onLoad, true);
-    openConsole(null, function(aHud) {
-      hud = aHud;
-      testDriver = testGen();
-      testNext();
-    });
-  }, true);
-}
+  Task.spawn(runner).then(finishTest);
 
-function testNext() {
-  testDriver.next();
-}
+  function* runner() {
+    let {tab} = yield loadTab(TEST_URI);
+    let hud = yield openConsole(tab);
+    let outputNode = hud.outputNode;
 
-function testGen() {
-  outputNode = hud.outputNode;
+    hud.jsterm.clearOutput();
 
-  hud.jsterm.clearOutput();
-
-  content.console.group("bug664131a");
+    content.console.group("bug664131a");
 
-  waitForMessages({
-    webconsole: hud,
-    messages: [{
-      text: "bug664131a",
-      consoleGroup: 1,
-    }],
-  }).then(testNext);
+    yield waitForMessages({
+      webconsole: hud,
+      messages: [{
+        text: "bug664131a",
+        consoleGroup: 1,
+      }],
+    });
 
-  yield undefined;
-
-  content.console.log("bug664131a-inside");
+    content.console.log("bug664131a-inside");
 
-  waitForMessages({
-    webconsole: hud,
-    messages: [{
-      text: "bug664131a-inside",
-      category: CATEGORY_WEBDEV,
-      severity: SEVERITY_LOG,
-      groupDepth: 1,
-    }],
-  }).then(testNext);
+    yield waitForMessages({
+      webconsole: hud,
+      messages: [{
+        text: "bug664131a-inside",
+        category: CATEGORY_WEBDEV,
+        severity: SEVERITY_LOG,
+        groupDepth: 1,
+      }],
+    });
 
-  yield undefined;
+    content.console.groupEnd("bug664131a");
+    content.console.log("bug664131-outside");
 
-  content.console.groupEnd("bug664131a");
-  content.console.log("bug664131-outside");
+    yield waitForMessages({
+      webconsole: hud,
+      messages: [{
+        text: "bug664131-outside",
+        category: CATEGORY_WEBDEV,
+        severity: SEVERITY_LOG,
+        groupDepth: 0,
+      }],
+    });
 
-  waitForMessages({
-    webconsole: hud,
-    messages: [{
-      text: "bug664131-outside",
-      category: CATEGORY_WEBDEV,
-      severity: SEVERITY_LOG,
-      groupDepth: 0,
-    }],
-  }).then(testNext);
-
-  yield undefined;
-
-  content.console.groupCollapsed("bug664131b");
-
-  waitForMessages({
-    webconsole: hud,
-    messages: [{
-      text: "bug664131b",
-      consoleGroup: 1,
-    }],
-  }).then(testNext);
+    content.console.groupCollapsed("bug664131b");
 
-  yield undefined;
+    yield waitForMessages({
+      webconsole: hud,
+      messages: [{
+        text: "bug664131b",
+        consoleGroup: 1,
+      }],
+    });
 
-  // Test that clearing the console removes the indentation.
-  hud.jsterm.clearOutput();
-  content.console.log("bug664131-cleared");
+    // Test that clearing the console removes the indentation.
+    hud.jsterm.clearOutput();
+    content.console.log("bug664131-cleared");
 
-  waitForMessages({
-    webconsole: hud,
-    messages: [{
-      text: "bug664131-cleared",
-      category: CATEGORY_WEBDEV,
-      severity: SEVERITY_LOG,
-      groupDepth: 0,
-    }],
-  }).then(testNext);
-
-  yield undefined;
-
-  testDriver = hud = null;
-  finishTest();
-
-  yield undefined;
+    yield waitForMessages({
+      webconsole: hud,
+      messages: [{
+        text: "bug664131-cleared",
+        category: CATEGORY_WEBDEV,
+        severity: SEVERITY_LOG,
+        groupDepth: 0,
+      }],
+    });
+  }
 }
-
--- a/browser/devtools/webconsole/test/head.js
+++ b/browser/devtools/webconsole/test/head.js
@@ -29,16 +29,19 @@ const CATEGORY_SECURITY = 6;
 const SEVERITY_ERROR = 0;
 const SEVERITY_WARNING = 1;
 const SEVERITY_INFO = 2;
 const SEVERITY_LOG = 3;
 
 // The indent of a console group in pixels.
 const GROUP_INDENT = 12;
 
+// The default indent in pixels, applied even without any groups.
+const GROUP_INDENT_DEFAULT = 6;
+
 const WEBCONSOLE_STRINGS_URI = "chrome://browser/locale/devtools/webconsole.properties";
 let WCU_l10n = new WebConsoleUtils.l10n(WEBCONSOLE_STRINGS_URI);
 
 function log(aMsg)
 {
   dump("*** WebConsoleTest: " + aMsg + "\n");
 }
 
@@ -1129,20 +1132,20 @@ function waitForMessages(aOptions)
     if ("repeats" in aRule) {
       let repeats = aElement.querySelector(".message-repeats");
       if (!repeats || repeats.getAttribute("value") != aRule.repeats) {
         return false;
       }
     }
 
     if ("groupDepth" in aRule) {
-      let timestamp = aElement.querySelector(".timestamp");
-      let indent = (GROUP_INDENT * aRule.groupDepth) + "px";
-      if (!timestamp || timestamp.style.marginRight != indent) {
-        is(timestamp.style.marginRight, indent,
+      let icon = aElement.querySelector(".icon");
+      let indent = (GROUP_INDENT * aRule.groupDepth + GROUP_INDENT_DEFAULT) + "px";
+      if (!icon || icon.style.marginLeft != indent) {
+        is(icon.style.marginLeft, indent,
            "group depth check failed for message rule: " + displayRule(aRule));
         return false;
       }
     }
 
     if ("longString" in aRule) {
       let longStrings = aElement.querySelectorAll(".longStringEllipsis");
       if (aRule.longString != !!longStrings[0]) {
--- a/browser/devtools/webconsole/webconsole.js
+++ b/browser/devtools/webconsole/webconsole.js
@@ -139,16 +139,19 @@ const MAX_HTTP_ERROR_CODE = 599;
 
 // Constants used for defining the direction of JSTerm input history navigation.
 const HISTORY_BACK = -1;
 const HISTORY_FORWARD = 1;
 
 // The indent of a console group in pixels.
 const GROUP_INDENT = 12;
 
+// The default indent in pixels, applied even without any groups.
+const GROUP_INDENT_DEFAULT = 6;
+
 // The number of messages to display in a single display update. If we display
 // too many messages at once we slow the Firefox UI too much.
 const MESSAGES_IN_INTERVAL = DEFAULT_LOG_LIMIT;
 
 // The delay between display updates - tells how often we should *try* to push
 // new messages to screen. This value is optimistic, updates won't always
 // happen. Keep this low so the Web Console output feels live.
 const OUTPUT_INTERVAL = 50; // milliseconds
@@ -2414,16 +2417,20 @@ WebConsoleFrame.prototype = {
     }
 
     // Make the icon container, which is a vertical box. Its purpose is to
     // ensure that the icon stays anchored at the top of the message even for
     // long multi-line messages.
     let iconContainer = this.document.createElementNS(XHTML_NS, "span");
     iconContainer.className = "icon";
 
+    // Apply the current group by indenting appropriately.
+    let iconMarginLeft = this.groupDepth * GROUP_INDENT + GROUP_INDENT_DEFAULT;
+    iconContainer.style.marginLeft = iconMarginLeft + "px";
+
     // Create the message body, which contains the actual text of the message.
     let bodyNode = this.document.createElementNS(XHTML_NS, "span");
     bodyNode.className = "body devtools-monospace";
 
     // Store the body text, since it is needed later for the variables view.
     let body = aBody;
     // If a string was supplied for the body, turn it into a DOM node and an
     // associated clipboard string now.
@@ -2471,18 +2478,16 @@ WebConsoleFrame.prototype = {
       repeatNode.textContent = 1;
       repeatNode._uid = [bodyNode.textContent, aCategory, aSeverity, aLevel,
                          aSourceURL, aSourceLine].join(":");
     }
 
     // Create the timestamp.
     let timestampNode = this.document.createElementNS(XHTML_NS, "span");
     timestampNode.className = "timestamp devtools-monospace";
-    // Apply the current group by indenting appropriately.
-    timestampNode.style.marginRight = this.groupDepth * GROUP_INDENT + "px";
 
     let timestampString = l10n.timestampString(timestamp);
     timestampNode.textContent = timestampString + " ";
 
     // Create the source location (e.g. www.example.com:6) that sits on the
     // right side of the message, if applicable.
     let locationNode;
     if (aSourceURL && IGNORED_SOURCE_URLS.indexOf(aSourceURL) == -1) {