955820 - console.group doesn't group messages when timestamps are disabled (i.e. by default)

Status: VERIFIED FIXED

(DevTools :: Console, defect)

Description

[Nickolay_Ponomarev]

Attachment: console.group-good.png

Run this example from MDN in the Tools -> Web Developer -> Web console:

console.log("This is the outer level");
console.group();
console.log("Level 2");
console.group();
console.log("Level 3");
console.warn("More of level 3");
console.groupEnd();
console.log("Back to level 2");
console.groupEnd();
console.debug("Back to the outer level");

If "Enable timestamps" in "Toolbox options -> Web Console" (in the gear button to the left of the console tab) is disabled, there's no grouping.

bug 722267 implemented the option and made it the default.

Thus in the default configuration, console.group() doesn't appear to work.

Comment 1

[Nickolay_Ponomarev]

Attachment: console.group-bad.png

Comment 2

[Nickolay_Ponomarev]

regression range from mozregression, just in case: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2eaab43b65d4&tochange=7b014f0f3b03

Comment 3

[Andrei Oprea [:andreio]]

Does the grouping rely on information from the timestamps? Disabling timestamps simply changes to 'display none'

Comment 4

[Mihai Sucan [:msucan]]

Attachment: bug955820-1.diff

console.group() has no effect since we disabled timestamps by default.

This is something we might want to uplift to aurora.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=204caefe9e3f

Comment 5

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 8377754 [details] [diff] [review]
bug955820-1.diff

Review of attachment 8377754 [details] [diff] [review]:
-----------------------------------------------------------------

haven't looked at what this looks like yet, but seems reasonable.

::: browser/devtools/webconsole/test/browser_webconsole_bug_664131_console_group.js
@@ +79,1 @@
>  }

nice. much better test using task.jsm.

Comment 6

[Mihai Sucan [:msucan]]

Thanks for the review.

Landed the patch: https://hg.mozilla.org/integration/fx-team/rev/393c539741f2

Comment 7

[Mihai Sucan [:msucan]]

Comment on attachment 8377754 [details] [diff] [review]
bug955820-1.diff

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 722267
User impact if declined: console.group() will not have any effect if the web developer tries to use this API when timestamps are not enabled in the web console output (the default).
Testing completed (on m-c, etc.): landed in fx-team, green try push.
Risk to taking this patch (and alternatives if risky): minimal, style changes only.
String or IDL/UUID changes made by this patch: none.

Thank you!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/393c539741f2

Comment 9

[Mihai Sucan [:msucan]]

checkin-needed for aurora. Thank you Sylvestre!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

You don't need to set checkin-needed on uplifts :)

https://hg.mozilla.org/releases/mozilla-aurora/rev/801ada2044a2

Comment 11

[Andrei Vaida [:avaida]]

I was able to confirm the fix for this issue on Mac OS X 10.9, using the description from Comment 0 with:
- the latest Beta (Build ID: 20140318013849) [1],
- the latest Aurora (Build ID: 20140324004000) [2]. 

1. Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
2. Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0