1200551 - console.log() formatted output applies only the second specified style when %c%c is part of the string

Status: VERIFIED FIXED

(DevTools :: General, defect, P2)

Description

[Pascal Chevrel:pascalc (PTO until April 26)]

Attachment: Testcase exposing the bug

The output of Monolog (de facto standard library for PHP development) to the browser is styled incorrectly in Firefox integrated devtools while it is styled correctly in Chrome, Firebug or Safari.

I am attaching to this bug a reduced testcase exposing the incorrect formatting in our devtools. This same testcase is also online there:
http://dev.pascalc.net/console_log_output/example.html

Here is the output of Firefox devtools:
http://dev.pascalc.net/console_log_output/firefox_console_output.png

Here is the output of Chrome:
http://dev.pascalc.net/console_log_output/chrome_console_output.png

Here is the output of Firebug:
http://dev.pascalc.net/console_log_output/firebug_console_output.png

I do not have Safari but a colleague told me that the output of their devtools is the same as Chrome and Firebug on this testcase.

I can reproduce this bug both on the release channel (42) and on Nightly.

Comment 1

[Pascal Chevrel:pascalc (PTO until April 26)]

Attachment: Firefox devtools output

Comment 2

[Pascal Chevrel:pascalc (PTO until April 26)]

Attachment: Firebug console output

Comment 3

[Pascal Chevrel:pascalc (PTO until April 26)]

Attachment: Chrome console output

Comment 4

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Pascal - excellent bug report, thank you!

Brian: good next bug for someone?

Comment 5

[Brian Grinstead [:bgrins]]

The issue seems to be with two %c strings next to each other.  For instance, I believe it styles as expected if I change:

From "%c%cgeneral%c %cINFO%c Information message sent to the console"
To "%c %cgeneral%c %cINFO%c Information message sent to the console"

i.e.

console.log(
    "%c %cgeneral%c %cINFO%c Information message sent to the console",
    "font-weight: normal",
    "font-weight: bold",
    "font-weight: normal",
    "background-color: blue; color: white; border-radius: 3px; padding: 0 2px 0 2px",
    "font-weight: normal"
    );

Comment 6

[Brian Grinstead [:bgrins]]

Simple test case:

console.log("%c%chi", "color: green", "background: red")

Should have both a green color and red background but in Firefox it only has green color.  At least, I'm guessing that's what it should be doing - this behavior is poorly specced, but we may as well match what other tools are doing.

Comment 7

[Brian Grinstead [:bgrins]]

The 'styles' packet coming from the server only includes the last one in this case.  Which points it to probably being an issue with Console.cpp parsing out the string

Comment 8

[Brian Grinstead [:bgrins]]

I'm pretty sure I tracked down the issue

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1200551 - Handle multiple %c formatters without a string between them by using only the last one for styling;r=bz,r=past

Bug 1200551 - Handle multiple %c formatters without a string between them by using only the last one for styling;r=bz,r=past

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> Created attachment 8659358 [details]
> MozReview Request: Bug 1200551 - Handle multiple %c formatters without a
> string between them by using only the last one for styling;r=bz,r=past
> 
> Bug 1200551 - Handle multiple %c formatters without a string between them by
> using only the last one for styling;r=bz,r=past

Unfortunately there isn't a spec that I know of for %c handling, but based on what other devtools are doing and what some tools generate (see Comment 0), I think this is the de facto right way to handle the case of:

console.log("%c%chi", "color: green", "background: red")  <-- Should be `background: red` but we currently show it as `color: green`

I've added a test in the patch that handles a variety of %c cases and makes sure it's parsed out as expected

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8659358 [details]
MozReview Request: Bug 1200551 - Handle multiple %c formatters without a string between them by using only the last one for styling;r=bz,r=past

Andrea knows this code much better than I do...

Comment 12

[Andrea Marchesini [:baku]]

Comment on attachment 8659358 [details]
MozReview Request: Bug 1200551 - Handle multiple %c formatters without a string between them by using only the last one for styling;r=bz,r=past

https://reviewboard.mozilla.org/r/18793/#review16823

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8659358 [details]
MozReview Request: Bug 1200551 - Handle multiple %c formatters without a string between them by using only the last one for styling;r=bz,r=past

https://reviewboard.mozilla.org/r/18793/#review16889

Yeah, let's go with the flow. If you feel ike it, a PR against https://github.com/DeveloperToolsWG/console-object/blob/master/api.md#format-specifiers to explicitly mention this case would be useful.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ea0f3fa6453f

Comment 15

[Brian Grinstead [:bgrins]]

(In reply to Panos Astithas [:past] from comment #13)
> Comment on attachment 8659358 [details]
> MozReview Request: Bug 1200551 - Handle multiple %c formatters without a
> string between them by using only the last one for styling;r=bz,r=past
> 
> https://reviewboard.mozilla.org/r/18793/#review16889
> 
> Yeah, let's go with the flow. If you feel ike it, a PR against
> https://github.com/DeveloperToolsWG/console-object/blob/master/api.md#format-
> specifiers to explicitly mention this case would be useful.

Alright, I'll do that

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ea0f3fa6453f

Comment 17

[Pascal Chevrel:pascalc (PTO until April 26)]

Thanks guys, I confirm that it works now correctly in today's nightly :)

Comment 18

[Brian Grinstead [:bgrins]]

Submitted a PR to the console-object repo: https://github.com/DeveloperToolsWG/console-object/pull/34