1593933 - Navigating should close existing console.group

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce

1. Navigate to data:text/html,<script>console.group("hello")</script>
2. Open the console
3. Ensure the "Persist Logs" setting is ON
4. Reload the tab several time

Actual results

The existing groups never close, which looks quite weird:

┌────────────────────────────────────────┐
│  ▼ hello                               │
├──┬─────────────────────────────────────┤
│  │ Navigated to data:text/html         │
├──┼─────────────────────────────────────┤
│  │  ▼ hello                            │
├──┴──┬──────────────────────────────────┤
│     │ Navigated to data:text/html      │
├─────┼──────────────────────────────────┤
│     │▼ hello                           │
└─────┴──────────────────────────────────┘

Comment 1

[Kartikeya Gokhale [:kgokhale]]

Hey I just tried replicating this error in the latest pull of the mozilla-central and wasn't able to replicate this issue. The logs groups didn't persist

Here's the screenshot

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Yes, it looks like bugzilla encoded some chars in Comment 0.

You can reproduce by evaluating console.group("hello"); document.location.reload(); a few time, having the "persist logs" option ON.
Let me know if you can reproduce.

I also assigned you the bug :)

Comment 3

[Kartikeya Gokhale [:kgokhale]]

Attachment: console.group bug

Hey I tried recreating that bug however wasn't able to do it.
I've attached the screenshot. Am I doing something wrong?

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Go to https://unclosed-console-group.glitch.me/ , open the console, and reload the page several times. It should reproduce.

Comment 5

[Kartikeya Gokhale [:kgokhale]]

Yeah it is showing now.
Could you please guide me more regarding where I would be able to fix this? and what all tests should be added/changed?
Thanks

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

So this is the piece of state that holds the current group: devtools/client/webconsole/reducers/messages.js#100-101
This needs to be set to null when the "Navigated to" message is added. The message is added from devtools/client/webconsole/webconsole-wrapper.js#257-258 , in the end, this will cause this redux action to be dispatched devtools/client/webconsole/actions/messages.js#50-53 .
This action is then handled in the messages reducer devtools/client/webconsole/reducers/messages.js#393
What we need to do is to check the type, and if it matches the navigation message type, set the currentGroup property to null.

Comment 7

[Kartikeya Gokhale [:kgokhale]]

Attachment: Fixed bug 1593933 - Navigating should close existing console.group

When persist log is on and the page is reloaded, if the console.group command is used, the newer logs keep on getting appended to the older ones.
Solved this by looking for the navigation marker message and when intercepted, sent a synthetic endGroup message to close the current group.

Comment 8

[Kartikeya Gokhale [:kgokhale]]

Attachment: Fixed bug 1593933 - Navigating should close existing console.group

Bug 1593933 - Navigating should close existing console.group

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Kartikeya, are you still interested in fixing this bug? There is a review comment waiting for your action.

Honza

Comment 10

[Kartikeya Gokhale [:kgokhale]]

I apologize for the delay but currently I wont be able to work on this. :(

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Thanks for the heads-up Kartikeya, I'll unassign the bug :)

Comment 12

[Miguel Roncancio]

I'd like to work on this!

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Thanks Miguel, it's now yours :)

Comment 14

[Miguel Roncancio]

Attachment: Bug 1593933 - [devtools] Navigating should close existing console.group messages. r=nchevobbe

change state property currentGroup

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdf395992b18
[devtools] Navigating should close existing console.group messages. r=nchevobbe

Comment 16

[Andreea Pavel [:apavel]]

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