Closed Bug 1306921 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Networking, defect)

47 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: valentin, Assigned: valentin)

Details

(Whiteboard: [necko-active])

Attachments

(1 file, 1 obsolete file)

No description provided.
Attachment #8796892 - Flags: review?(jaws)
Is there a new patch coming here or is the one attached the one you wanted me to review?
New patch is incoming.
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! :)
Attachment #8796892 - Attachment is obsolete: true
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.
Attachment #8798440 - Flags: review+
(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.
https://hg.mozilla.org/mozilla-central/rev/651f1169e0f9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.