Closed Bug 884750 Opened 10 years ago Closed 10 years ago

Persistent logs options should apply instantly

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: Optimizer, Assigned: dorian)

Details

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

Attachments

(1 file)

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
Priority: -- → P3
Whiteboard: [good first bug][lang=js][mentor=msucan]
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)
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
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 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+
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=).
I have an orange on Win7 opt (bc).
Unfortunately I don't understand how the log relate to my changes. Have I break something ?
(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]
s/god/good
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
Closed: 10 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
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.