801067 - The Web Console should highlight the categories when new messages appear in them

Status: RESOLVED DUPLICATE of bug 879367

(DevTools :: Console, defect, P3)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Today I was using the Web Console and trying to test some one-line JS scripts.

For example, I tried the following scripts:
> let a = [1,2,3];
> for each (let i in a) { console.log(i); }

This then showed "undefined" but I never saw the logging messages.

At first I thought that maybe log messages originating from the Web Console might not show up in the Web Console, but then I remembered! A couple days earlier I had unchecked Logging messages in the Web Console. Ah, that was it!

So long story short, I think we should highlight the appropriate Filter message when one gets filtered. This would have shown me that new logging messages were incoming but getting blocked. This could be a simple glow so it's not annoying but it could do a single pulse for each log message.

Comment 1

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

nice idea.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: WIP patch (doesn't work correctly)

This is a work-in-progress patch that is rough and doesn't work correctly, but might be close to what is desired.

There is a bug in the code to determine if there are any hidden nodes available. I think once that is fixed it should work.

It would be nice to tweak it so it doesn't need to check all four filters each time that the messages are appended to the console.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Mihai, what do you think about this?

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 712817 [details] [diff] [review]
Patch

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

Thank you for the patch! This looks good.

Problem #1: button highlights show when they shouldn't. Open the web console, open a web page, so you have network messages shown. Hide network messages. Then eval console.log("foo"). Now the "net" button is highlighted. I would expect it is highlighted only for new messages, only if some new net message comes in.

Problem #2: I'm concerned by the performance implications of this patch. You added 4 calls to _updateToolbarbuttonHighlight(), each time messages are flushed to the output. Each call to _updateToolbarbuttonHighlight() does one expensive call to querySelectorAll() and one call to querySelector(). This is quite a lot in a rush of messages coming from a web-based game.

I suggest that the way you track if a new message is available avoids querySelectorAll() entirely: you can keep a count of new messages per-category. This count would be reset every time you enable the filter for that category. This would fix both of the problems mentioned above. Also, you should only call _updateToolbarbuttonHighlight() at the end of a queue - see when regroupOutput() is invoked.

I'm looking forward for the updated patch. Thanks!

::: browser/devtools/webconsole/webconsole.js
@@ +608,5 @@
>        Services.prefs.clearUserPref("devtools.webconsole.fontSize");
>      }
>    },
>  
> +  _updateToolbarbuttonHighlight: function(aCategory, aNodeMsgType) {

nits: please follow the coding style of the file:

- always add a function name, here WCF__update...
- add a comment that describes the function.
- always use curly brackets for conditions and loops.

@@ +658,5 @@
>          let state = target.getAttribute("checked") !== "true";
>          target.setAttribute("checked", state);
>  
> +        if (state)
> +          target.removeAttribute("highlight");

Ditto.

::: browser/themes/gnomestripe/devtools/webconsole.css
@@ +113,5 @@
>    display: none;
>  }
>  
> +.webconsole-filter-button[highlight]:not(:active) {
> +  box-shadow: 0 0 3px rgb(159,223,255);

I'd like the highlight to be slightly more obvious. Maybe you should ask Paul if he has any preferences.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Fixing bug 879367 would be a better solution and would obsolete this bug.