1403977 - Multiple styled messages lose line breaks

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Greg Tatum [:gregtatum]]

Attachment: Screen Shot 2017-09-28 at 10.33.25 AM.png

Doing multiple %c styles in a console.log message does not use the line breaks. This is particularly bad in a library that spits out nicely formatted messages.

Correct:
  console.log(
    "%cfoo \n%cbar"
  )
  ➡ %cfoo 
  ➡ %cbar

Correct:
  console.log(
    "%cfoo \nbar",
    "color: red"
  )
  ➡ foo 
  ➡ bar

Incorrect:
  console.log(
    "%cfoo \n%cbar",
    "color: red", "color: blue"
  )
  ➡ foo bar

Expected behavior
  console.log(
    "%cfoo \n%cbar",
    "color: red", "color: blue"
  )
  ➡ foo
  ➡ bar

Comment 1

[Brian Grinstead [:bgrins]]

This is new frontend / reps specific - the packet looks fine and it renders as expected in the Browser Console

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1403977 - [WIP] Wrap reps in a non-flex layout parent.

Review commit: https://reviewboard.mozilla.org/r/191534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/191534/

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: newline-params.png

When looking at this I found another weird case without custom styling:

console.log(
    "foo",
    "bar"
  )

Logs bar before foo

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Created attachment 8925715 [details]
> newline-params.png
> 
> When looking at this I found another weird case without custom styling:
> 
> console.log(
>     "foo",
>     "bar"
>   )
> 
> Logs bar before foo

"bar" is not really before foo, the html element *is* after bar, but it is aligned top : 
```
+----+-----+
|"   |"bar"|
|foo"|     |
+----+-----+

```

Adding `align-items: end;` in http://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/devtools/client/themes/webconsole.css#925-929 fix this issue, but we have to check that it does not affect other types of messages.

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: Bug 1403977 - Show taller toolbox for tool-specific screenshots;

Review commit: https://reviewboard.mozilla.org/r/198194/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198194/

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: Bug 1403977 - Switch to inline-block for message-body and children;

Review commit: https://reviewboard.mozilla.org/r/198258/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198258/

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8926943 [details]
Bug 1403977 - Show taller toolbox for tool-specific screenshots;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198194/diff/1-2/

Comment 8

[Brian Grinstead [:bgrins]]

Nicolas, this is more of a feedback request as I haven't done a lot of manual testing beyond the fix for this bug and am still awaiting screenshot results. If you notice any logs that are broken let me know and I'll add it into the screenshots job.

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8926943 [details]
Bug 1403977 - Show taller toolbox for tool-specific screenshots;

https://reviewboard.mozilla.org/r/198194/#review203500

Thanks

Comment 10

[Brian Grinstead [:bgrins]]

Screenshots with the first patch (expanded toolbox): https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=2bdf6eed0f64a51dfe099e089852533595f1a896&newProject=try&newRev=54e674a1241b6bc2055cc4ce60da0225556446f9&filter=devtools

Screenshots comparing first and second patch (fixing newline handling): https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=775c920599604320a142f7b0a7e1388e540be6f6&newProject=try&newRev=54e674a1241b6bc2055cc4ce60da0225556446f9&filter=devtools
- Specifically: https://screenshots.mattn.ca/comparisons/try/775c920599604320a142f7b0a7e1388e540be6f6/try/54e674a1241b6bc2055cc4ce60da0225556446f9/windows10-64/devtools_5_webconsole.png

Comment 11

[Brian Grinstead [:bgrins]]

Not sure what's going on with the Windows 7 changes in the second push - it looks like the two builds were running with different system themes

Comment 12

[Brian Grinstead [:bgrins]]

Comment on attachment 8926943 [details]
Bug 1403977 - Show taller toolbox for tool-specific screenshots;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198194/diff/2-3/

Comment 13

[Brian Grinstead [:bgrins]]

Comment on attachment 8927046 [details]
Bug 1403977 - Switch to inline-block for message-body and children;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198258/diff/1-2/

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8927046 [details]
Bug 1403977 - Switch to inline-block for message-body and children;

https://reviewboard.mozilla.org/r/198258/#review204056

Did some testing and everything seems good

Comment 15

[Brian Grinstead [:bgrins]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #14)
> Did some testing and everything seems good

Thanks for testing. Let's land it after the merge today just in case something comes up.

Comment 16

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=410a4fc3a23d04ce7b496794a33bed4ce8723270

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45ee82268d2d
Show taller toolbox for tool-specific screenshots;r=MattN
https://hg.mozilla.org/integration/autoland/rev/43948076a0a6
Switch to inline-block for message-body and children;r=nchevobbe

Comment 18

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/45ee82268d2d
https://hg.mozilla.org/mozilla-central/rev/43948076a0a6