Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20388 closed defect (fixed)

Consolidate prefs usage in torbutton.js

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Minor Keywords: tbb-code-cleanup, TorBrowserTeam201610R
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In torbutton.js, we call

var pref_service = Components.classes["@mozilla.org/preferences-service;1"]
    .getService(Components.interfaces.nsIPrefBranch);

or

    m_tb_prefs =  Components.classes["@mozilla.org/preferences-service;1"]
        .getService(Components.interfaces.nsIPrefBranch);

7 times when that functionality is already available in Services.prefs. So I am proposing to consolidate this.

Child Tickets

Change History (8)

comment:1 Changed 3 years ago by arthuredelstein

Keywords: TorBrowserTeam201610R added
Status: newneeds_review

comment:2 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks, applied to master (commit 2cef75462d4a7f640b4586c0cd50b6679b9fc7ae).

comment:3 Changed 3 years ago by bugzilla

Minor thing about prefs usage in torbutton.js: extensions.torbutton.lastUpdateCheck is set to 1475974589.6 instead of 1475974589600.

comment:4 in reply to:  3 ; Changed 3 years ago by mcs

Replying to bugzilla:

Minor thing about prefs usage in torbutton.js: extensions.torbutton.lastUpdateCheck is set to 1475974589.6 instead of 1475974589600.

Does this cause buggy behavior or is it just surprising that Date.now() / 1000 is stored without rounding to an integer?

comment:5 in reply to:  4 ; Changed 3 years ago by bugzilla

Replying to mcs:

Replying to bugzilla:

Minor thing about prefs usage in torbutton.js: extensions.torbutton.lastUpdateCheck is set to 1475974589.6 instead of 1475974589600.

Does this cause buggy behavior or is it just surprising that Date.now() / 1000 is stored without rounding to an integer?

All other prefs with lastUpdate use integers. If you treat e.g. milliseconds as microseconds somewhere, it may lead to some surprises.

Last edited 3 years ago by bugzilla (previous) (diff)

comment:6 in reply to:  5 ; Changed 3 years ago by arthuredelstein

Replying to bugzilla:

Replying to mcs:

Replying to bugzilla:

Minor thing about prefs usage in torbutton.js: extensions.torbutton.lastUpdateCheck is set to 1475974589.6 instead of 1475974589600.

Does this cause buggy behavior or is it just surprising that Date.now() / 1000 is stored without rounding to an integer?

All other prefs with lastUpdate use integers. If you treat e.g. milliseconds as microseconds somewhere, it may lead to some surprises.

I would like to keep a new issue such as this one in a separate ticket so we don't lose it. That being said, all "lastUpdate" prefs on my TBB are in units of seconds, either rounded or not. What is unusual is that this is a string pref instead of an integer pref, but I don't think that is a problem.

comment:7 in reply to:  6 ; Changed 3 years ago by bugzilla

Replying to arthuredelstein:

I would like to keep a new issue such as this one in a separate ticket so we don't lose it.

OK, so you mean that TBB Team prefer anybody filing a new ticket for even minor inconsistencies, so that it'll look like a Q & A site with separate discussion for every question. Right?

That being said, all "lastUpdate" prefs on my TBB are in units of seconds, either rounded or not. What is unusual is that this is a string pref instead of an integer pref, but I don't think that is a problem.

Mozilla obviously uses string prefs for reals and big integers. And your pref is a string, so it points to it is expected to be a big int, perhaps, for milliseconds.

comment:8 in reply to:  7 Changed 3 years ago by arthuredelstein

Replying to bugzilla:

Replying to arthuredelstein:

I would like to keep a new issue such as this one in a separate ticket so we don't lose it.

OK, so you mean that TBB Team prefer anybody filing a new ticket for even minor inconsistencies, so that it'll look like a Q & A site with separate discussion for every question. Right?

Yes, absolutely. If the issue is unrelated to the subject of the ticket, then it will be overlooked because it won't appear as a separate item in query results. Especially if the ticket, like this one, is already closed. "Out of sight, out of mind." The rule is: one ticket per issue.

Note: See TracTickets for help on using tickets.