Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Persistent logs options should apply instantly

RESOLVED FIXED in Firefox 25

Status

()

Firefox
Developer Tools: Console
P3
enhancement
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Optimizer, Assigned: dorian jaminais)

Tracking

Trunk
Firefox 25
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js][mentor=msucan])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
Right now, when you change the "Enable persistent logs" options from options panel, it does not get applied until toolbox is reopened. This can be made instant if someone is listening to "pref-changed" event on gDevTools and acting accordingly.

See http://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/theme-switching.js#70

Updated

4 years ago
Priority: -- → P3
Whiteboard: [good first bug][lang=js][mentor=msucan]
(Assignee)

Comment 1

4 years ago
Created attachment 766453 [details] [diff] [review]
apply the log persistance immediately

This patch fixes the problem.

I did not find where and how to unit test this. Can you provide me some guidance on how to do this ?
Attachment #766453 - Flags: review?(mihai.sucan)
Flags: needinfo?(mihai.sucan)
(Reporter)

Comment 2

4 years ago
Comment on attachment 766453 [details] [diff] [review]
apply the log persistance immediately

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

Hi Dorian, thanks for taking this bug. The patch does what is intended, but things can be optimized here :)

Instead of querying the prefs each time, why not convert the getter to a normal property and update its value whenever the value gets changed via options panel.

That can be done via the event listener gDevTools.on("pref-changed", callback) and changing the value of _persistLog.

Take a look at hos its done at one place already : http://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/theme-switching.js#56
(Assignee)

Comment 3

4 years ago
I did it that way because the property is only used in this method. Yet you are right that this can be optimised because the method is called each time a tab is refreshed.

I'll update my patch as soon as possible and re-submit.

Thanks
Flags: needinfo?(mihai.sucan)

Comment 4

4 years ago
Comment on attachment 766453 [details] [diff] [review]
apply the log persistance immediately

There's no point of caching this pref. It's only accessed on page reload. Let's keep it simple.
Attachment #766453 - Flags: review?(mihai.sucan) → review+

Comment 5

4 years ago
Dorian, once this is green: https://tbpl.mozilla.org/?tree=Try&rev=c32ab4b1c7cd add [land-in-fx-team] in the whiteboard, and update your patch to reflect the review (r=paul - and BTW, it's r=, not ref=).
(Assignee)

Comment 6

4 years ago
I have an orange on Win7 opt (bc).
Unfortunately I don't understand how the log relate to my changes. Have I break something ?
(Reporter)

Comment 7

4 years ago
(In reply to dorian jaminais from comment #6)
> I have an orange on Win7 opt (bc).
> Unfortunately I don't understand how the log relate to my changes. Have I
> break something ?

Nope it is not related. All god to go here.
Whiteboard: [good first bug][lang=js][mentor=msucan] → [good first bug][lang=js][mentor=msucan][land-in-fx-team]
(Reporter)

Comment 8

4 years ago
s/god/good

Comment 9

4 years ago
I'm new to programming, and I want to help contribute to opensource projects. How can I help?
https://hg.mozilla.org/integration/fx-team/rev/708a370466ff
Assignee: nobody → dorian
Whiteboard: [good first bug][lang=js][mentor=msucan][land-in-fx-team] → [good first bug][lang=js][mentor=msucan][fixed-in-fx-team]
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/708a370466ff
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=js][mentor=msucan][fixed-in-fx-team] → [good first bug][lang=js][mentor=msucan]
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.