Bug 1438442 - Fix styled console.log issue with empty style; r=Honza. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 15 Feb 2018 09:09:36 +0100
changeset 756231 bf711afa76dd784d3cf1f973c47e4aa139d5bc5e
parent 756220 f3b184c83b063fdf1758b2ed4ecf57dd1b16641d
push id99429
push userbmo:nchevobbe@mozilla.com
push dateFri, 16 Feb 2018 16:36:48 +0000
reviewersHonza
bugs1438442
milestone60.0a1
Bug 1438442 - Fix styled console.log issue with empty style; r=Honza. It is totally valid to declare an empty style in styled console.log. But we were appending a space after a string with empty style because we were checking that the style wasn't falsy, and empty string is falsy. We fix that by checking against undefined. A test is added to make sure we don't regress that. MozReview-Commit-ID: EOQ49Gt0Cr9
devtools/client/webconsole/new-console-output/components/message-types/ConsoleApiCall.js
devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js
devtools/client/webconsole/new-console-output/test/fixtures/stubs/consoleApi.js
--- a/devtools/client/webconsole/new-console-output/components/message-types/ConsoleApiCall.js
+++ b/devtools/client/webconsole/new-console-output/components/message-types/ConsoleApiCall.js
@@ -147,20 +147,23 @@ function formatReps(options = {}) {
         loadedObjectProperties,
         loadedObjectEntries,
         type,
       }))
       // Interleave spaces.
       .reduce((arr, v, i) => {
         // We need to interleave a space if we are not on the last element AND
         // if we are not between 2 messages with user provided style.
-        const needSpace = i + 1 < parameters.length &&
-          (!userProvidedStyles || !userProvidedStyles[i] || !userProvidedStyles[i + 1]);
+        const needSpace = i + 1 < parameters.length && (
+          !userProvidedStyles
+          || userProvidedStyles[i] === undefined
+          || userProvidedStyles[i + 1] === undefined
+        );
 
         return needSpace
-          ? arr.concat(v, dom.span({}, " "))
+          ? arr.concat(v, " ")
           : arr.concat(v);
       }, [])
   );
 }
 
 module.exports = ConsoleApiCall;
 
--- a/devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
@@ -61,16 +61,36 @@ describe("ConsoleAPICall component:", ()
 
       const secondElementStyle = elements.eq(1).prop("style");
       // Allowed styles are applied accordingly on the second element.
       expect(secondElementStyle.color).toBe(`red`);
       // Forbidden styles are not applied.
       expect(secondElementStyle.background).toBe(undefined);
     });
 
+    it("renders custom styled logs with empty style as expected", () => {
+      const message = stubPreparedMessages.get('console.log("%cHello%c|%cWorld")');
+      const wrapper = render(ConsoleApiCall({ message, serviceContainer }));
+
+      const elements = wrapper.find(".objectBox-string");
+      expect(elements.text()).toBe("Hello|World");
+      expect(elements.length).toBe(3);
+
+      const firstElementStyle = elements.eq(0).prop("style");
+      // Allowed styles are applied accordingly on the first element.
+      expect(firstElementStyle.color).toBe("red");
+
+      const secondElementStyle = elements.eq(1).prop("style");
+      expect(secondElementStyle.color).toBe(undefined);
+
+      const thirdElementStyle = elements.eq(2).prop("style");
+      // Allowed styles are applied accordingly on the third element.
+      expect(thirdElementStyle.color).toBe("blue");
+    });
+
     it("renders repeat node", () => {
       const message = stubPreparedMessages.get("console.log('foobar', 'test')");
       const wrapper = render(ConsoleApiCall({
         message,
         serviceContainer,
         repeat: 107
       }));
 
--- a/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js
+++ b/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/stub-snippets.js
@@ -105,16 +105,27 @@ consoleApi.set("console.log(%cfoobar)", 
   keys: ["console.log(%cfoobar)"],
   code: `
 console.log(
   "%cfoo%cbar",
   "color:blue;font-size:1.3em;background:url('http://example.com/test');position:absolute;top:10px",
   "color:red;background:\\165rl('http://example.com/test')");
 `});
 
+consoleApi.set('console.log("%cHello%c|%cWorld")', {
+  keys: ['console.log("%cHello%c|%cWorld")'],
+  code: `
+  console.log(
+    "%cHello%c|%cWorld",
+    "color:red",
+    "",
+    "color: blue"
+  );
+`});
+
 consoleApi.set("console.group(%cfoo%cbar)", {
   keys: ["console.group(%cfoo%cbar)", "console.groupEnd(%cfoo%cbar)"],
   code: `
 console.group(
   "%cfoo%cbar",
   "color:blue;font-size:1.3em;background:url('http://example.com/test');position:absolute;top:10px",
   "color:red;background:\\165rl('http://example.com/test')");
 console.groupEnd();
--- a/devtools/client/webconsole/new-console-output/test/fixtures/stubs/consoleApi.js
+++ b/devtools/client/webconsole/new-console-output/test/fixtures/stubs/consoleApi.js
@@ -1117,16 +1117,48 @@ stubPreparedMessages.set(`console.log(%c
   "userProvidedStyles": [
     "color:blue;font-size:1.3em;background:url('http://example.com/test');position:absolute;top:10px",
     "color:red;background:url('http://example.com/test')"
   ],
   "notes": null,
   "indent": 0
 }));
 
+stubPreparedMessages.set(`console.log("%cHello%c|%cWorld")`, new ConsoleMessage({
+  "id": "1",
+  "allowRepeating": true,
+  "source": "console-api",
+  "timeStamp": 1518681614352,
+  "type": "log",
+  "helperType": null,
+  "level": "log",
+  "messageText": null,
+  "parameters": [
+    "Hello",
+    "|",
+    "World"
+  ],
+  "repeatId": "{\"frame\":{\"source\":\"http://example.com/browser/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/test-console-api.html\",\"line\":2,\"column\":3},\"groupId\":null,\"indent\":0,\"level\":\"log\",\"messageText\":null,\"parameters\":[\"Hello\",\"|\",\"World\"],\"source\":\"console-api\",\"type\":\"log\",\"userProvidedStyles\":[\"color:red\",\"\",\"color: blue\"]}",
+  "stacktrace": null,
+  "frame": {
+    "source": "http://example.com/browser/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/test-console-api.html",
+    "line": 2,
+    "column": 3
+  },
+  "groupId": null,
+  "exceptionDocURL": null,
+  "userProvidedStyles": [
+    "color:red",
+    "",
+    "color: blue"
+  ],
+  "notes": null,
+  "indent": 0
+}));
+
 stubPreparedMessages.set(`console.group(%cfoo%cbar)`, new ConsoleMessage({
   "id": "1",
   "allowRepeating": true,
   "source": "console-api",
   "timeStamp": 1502884924887,
   "type": "startGroup",
   "helperType": null,
   "level": "log",
@@ -2583,16 +2615,47 @@ stubPackets.set(`console.log(%cfoobar)`,
     ],
     "timeStamp": 1502884924883,
     "timer": null,
     "workerType": "none",
     "category": "webdev"
   }
 });
 
+stubPackets.set(`console.log("%cHello%c|%cWorld")`, {
+  "from": "server1.conn0.child1/consoleActor2",
+  "type": "consoleAPICall",
+  "message": {
+    "addonId": "",
+    "arguments": [
+      "Hello",
+      "|",
+      "World"
+    ],
+    "columnNumber": 3,
+    "counter": null,
+    "filename": "http://example.com/browser/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/test-console-api.html",
+    "functionName": "triggerPacket",
+    "groupName": "",
+    "level": "log",
+    "lineNumber": 2,
+    "prefix": "",
+    "private": false,
+    "styles": [
+      "color:red",
+      "",
+      "color: blue"
+    ],
+    "timeStamp": 1518681614352,
+    "timer": null,
+    "workerType": "none",
+    "category": "webdev"
+  }
+});
+
 stubPackets.set(`console.group(%cfoo%cbar)`, {
   "from": "server1.conn0.child1/consoleActor2",
   "type": "consoleAPICall",
   "message": {
     "addonId": "",
     "arguments": [
       "foo",
       "bar"