Closed
Bug 1278337
Opened 8 years ago
Closed 8 years ago
remove preprocessing from preferences .js files by converting to AppConstants.jsm
Categories
(Thunderbird :: Preferences, defect)
Thunderbird
Preferences
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 50.0
People
(Reporter: aceman, Assigned: aceman)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
Remove preprocessing from preferences .js files by converting to AppConstants.jsm
Attachment #8760464 -
Flags: review?(mkmelin+mozilla)
Comment 2•8 years ago
|
||
Comment on attachment 8760464 [details] [diff] [review]
patch
Review of attachment 8760464 [details] [diff] [review]:
-----------------------------------------------------------------
::: mail/components/preferences/advanced.js
@@ +77,5 @@
> + checkElem.checked = false;
> + }
> + checkElem = document.getElementById("checkDefaultButton");
> + if (checkElem)
> + checkElem.disabled = true;
Good with the added comment, but document.getElementById is really fast enough that the old code is better as it's more readable
@@ +208,5 @@
> */
> updateReadPrefs: function ()
> {
> + if (!AppConstants.MOZ_UPDATER)
> + return;
unneeded, no?
@@ +255,5 @@
> */
> updateWritePrefs: function ()
> {
> + if (!AppConstants.MOZ_UPDATER)
> + return;
and here
@@ +278,5 @@
>
> showUpdates: function ()
> {
> + if (!AppConstants.MOZ_UPDATER)
> + return;
and here
Attachment #8760464 -
Flags: review?(mkmelin+mozilla) → review+
(In reply to Magnus Melin from comment #2)
> @@ +208,5 @@
> > */
> > updateReadPrefs: function ()
> > {
> > + if (!AppConstants.MOZ_UPDATER)
> > + return;
>
> unneeded, no?
Safety and preserving semantics :)
With the preprocessing the functions wouldn't even exist. So now at least make them do nothing so that nobody calls them by mistake. And if he does, no harm happens.
Should I remove it?
Comment 4•8 years ago
|
||
Well they are available now, not just called. Anyway, I think it's unnecessary to have those checks
OK.
Attachment #8760464 -
Attachment is obsolete: true
Attachment #8760890 -
Flags: review+
Keywords: checkin-needed
Comment 6•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/c37795888b0e9e92bbb145403b72bf14a201285e
Bug 1278337 - remove preprocessing from preferences .js files by converting to AppConstants.jsm. r=mkmelin
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in
before you can comment on or make changes to this bug.
Description
•