1306921 - Add buttons to clear log file and log modules to about:networking

Status: RESOLVED FIXED

(Core :: Networking, defect)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add buttons to clear log file and log modules to about:networking

Comment 2

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

Is there a new patch coming here or is the one attached the one you wanted me to review?

Comment 3

[Valentin Gosu [:valentin] (he/him)]

New patch is incoming.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add buttons to start-stop logging to about:networking

I changed this to add start-stop buttons, to make the entire workflow easier, based on feedback from the necko team. Thanks for all the help with the reviews! :)

Comment 5

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

Comment on attachment 8798440 [details] [diff] [review]
Add buttons to start-stop logging to about:networking

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

::: toolkit/content/aboutNetworking.js
@@ +203,5 @@
>    updateLogModules();
> +
> +  // If we can't set either the file, or the modules at runtime,
> +  // the start and stop buttons wouldn't really do anything.
> +  if (setLogButton.disabled && setModulesButton.disabled) {

The comment and code here are inconsistent. The comment says "or" and the code says "and". The code should be updated to use "or".

@@ +302,4 @@
>  function setLogModules() {
> +  let setLogModulesButton = document.getElementById("set-log-modules-button");
> +  if (setLogModulesButton.disabled) {
> +    // The modules were set via env var, so we shouldn't try to change them.

It would be nice to include some note on the page explaining why these buttons are disabled.

Comment 6

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jared Wein [:jaws] (overloaded with reviews / please needinfo? me) from comment #5)

Thanks a lot for reviewing!

> > +    // The modules were set via env var, so we shouldn't try to change them.
> It would be nice to include some note on the page explaining why these
> buttons are disabled.

I hope to include most of this info on the MDN page that we link to.

Comment 7

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/651f1169e0f94c17df7020d9c5a7885ecdb64578
Bug 1306921 - Add buttons to start-stop logging to about:networking r=jaws

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/651f1169e0f9