Closed Bug 958850 Opened 6 years ago Closed 6 years ago

port firefox Advanced | Update UI changes to thunderbird (port bug 600505, bug 701987)

Categories

(Thunderbird :: Preferences, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: mkmelin, Assigned: AMiras)

Details

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

Attachments

(1 file, 1 obsolete file)

As suggested in bug Bug 707489, we should update our Advanced | Update UI to match what Firefox has.

We should port:
- Bug 600505 - App update preference ui favors disabling app update.
- Bug 701987 - Remove checkbox for add-on auto-checking for updates
(extensions.update.enabled) from the options window, and make enabling
extensions.update.autoUpdateDefault in the add-ons manager also enable
extensions.update.enabled.
Hello Magnus. I can also try to port these bugs.
Great!
Assignee: nobody → amarostegui
Several issues about this one:

1)

Thunderbird's function “updateModeItems” in “advanced.js” checks whether maintenance service (MOZ_MAINTENANCE_SERVICE) control should be shown. I don't know why, but Firefox function is different and this code is not there.

I've copied the code into the new function “updateReadPrefs”. I think it should be enough.

2)

In bug 701987 a change was made in the gears button menu of the Add-on manager ((…)/mozapps/extensions/content/extensions.js).

I guess Thunderbird uses this code too, because I haven't found a right place in “comm-central” to replicate this.

3)

Regarding the change made in bug 701987 (/toolkit/mozapps/extensions/content/extensions.js):

Shouldn't we get rid of the line “if (newvalue)”? We activate "extensions.update.enabled” if the user wants “extensions.update.autoUpdateDefault” on, but we don't deactivate it anymore if not. Why?

4)

Again, in bug 701987 a user asked to keep user interface to access “extensions.update.enabled”. The answer from UX team was that he can always go to “about:config” to change this.

I think we don't have this ability in Thunderbird so... How do we handle this situation?
Attachment #8361748 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8361748 [details] [diff] [review]
Bug 958850 - port firefox Advanced | Update UI changes to thunderbird (port bug 600505, bug 701987)

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

::: mail/components/preferences/advanced.xul
@@ +388,4 @@
>            <separator/>
>            <hbox>
>              <button id="showUpdateHistory"
>                      label="&showUpdates.label;" accesskey="&showUpdates.accesskey;"

This button is inside the groupbox in firefox. Lets do that too

::: mail/locales/en-US/chrome/messenger/preferences/advanced.dtd
@@ +71,5 @@
> +<!ENTITY updateManual.label              "Never check for updates (not recommended: security risk)">
> +<!ENTITY updateManual.accesskey          "N">
> +<!ENTITY updateAutoAddonWarn.label       "Warn me if this will disable any of my add-ons">
> +<!ENTITY updateAutoAddonWarn.accesskey   "W">
> +<!ENTITY showUpdates.label               "Show Update History">

firefox uses updateHistory.label
(In reply to Antonio Miras [:AMiras] from comment #3)
> Thunderbird's function “updateModeItems” in “advanced.js” checks whether
> maintenance service (MOZ_MAINTENANCE_SERVICE) control should be shown. I
> don't know why, but Firefox function is different and this code is not there.
> 
> I've copied the code into the new function “updateReadPrefs”. I think it
> should be enough.

AFAIU, this should be fine yes.

> 2)
> 
> In bug 701987 a change was made in the gears button menu of the Add-on
> manager ((…)/mozapps/extensions/content/extensions.js).
> 
> I guess Thunderbird uses this code too, because I haven't found a right
> place in “comm-central” to replicate this.

Yes, everything in mozapps/ (and toolkit in general) is shared code

> 3)
> 
> Regarding the change made in bug 701987
> (/toolkit/mozapps/extensions/content/extensions.js):
> 
> Shouldn't we get rid of the line “if (newvalue)”? We activate
> "extensions.update.enabled” if the user wants
> “extensions.update.autoUpdateDefault” on, but we don't deactivate it anymore
> if not. Why?

That code is different now, please see the current tip.

> 4)
> 
> Again, in bug 701987 a user asked to keep user interface to access
> “extensions.update.enabled”. The answer from UX team was that he can always
> go to “about:config” to change this.
> 
> I think we don't have this ability in Thunderbird so... How do we handle
> this situation?

In thunderbird you can do the same. It's the Config Editor you find under Preferences | Advanced
OK. How about this?
Attachment #8361748 - Attachment is obsolete: true
Attachment #8361748 - Flags: review?(mkmelin+mozilla)
Attachment #8364939 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8364939 [details] [diff] [review]
Rev1 - Bug 958850 - port firefox Advanced | Update UI changes to thunderbird (port bug 600505, bug 701987) Bug958850

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

Looks good -thx for the patch, Antonio! r=mkmelin
Attachment #8364939 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/c1f8875ca4cd
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in before you can comment on or make changes to this bug.