706292 - Hide the automatic update settings from the add-on details for the hotfix add-on

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Since it follows different settings there is no point in showing this UI to the users. See bug 694068 comment 7

Comment 1

[Matthew N. [:MattN]]

Attachment: WIP without tests

Is this the right approach or should I be using aIsRemote or adding something to the aAddon object to be reused?  I'll make tests once I know the preferred approach.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 616208 [details] [diff] [review]
WIP without tests

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

Yea, that looks good.

Since we're using that pref in a few places now, bonus points if you make the hotfix ID into a getter on AddonManager (ie, AddonManager.hotfixID) and use that here and the other places (AddonManager.jsm and content/update.js).

Comment 3

[Matthew N. [:MattN]]

Attachment: v.1 added AddonManager.hotfixID and tests

Added pref observer and hotfixID property to AddonManager and AddonManagerInternal like the other prefs.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 624199 [details] [diff] [review]
v.1 added AddonManager.hotfixID and tests

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

Could you add a test for AddonManager.hotfixID in toolkit/mozapps/extensions/test/xpcshell/test_pref_properties.js ?

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +408,5 @@
>  var gCheckUpdateSecurityDefault = true;
>  var gCheckUpdateSecurity = gCheckUpdateSecurityDefault;
>  var gUpdateEnabled = true;
>  var gAutoUpdateDefault = true;
> +var gHotfixID;

Nit: Explicitly set this to null here, to make it obvious that that's the expected default value.

@@ +1707,5 @@
> +  get hotfixID() {
> +    return gHotfixID;
> +  },
> +
> +  set hotfixID(aValue) {

Want to discourage this being set at runtime (while still technically supporting it), so I'd rather not have a setter for it.

Comment 5

[Matthew N. [:MattN]]

Attachment: v.2 Address review comments

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 624311 [details] [diff] [review]
v.2 Address review comments

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

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +2089,5 @@
> +  get hotfixID() {
> +    return AddonManagerInternal.hotfixID;
> +  },
> +
> +  set hotfixID(aValue) {

Forgot to remove this :)

Comment 7

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccb2effe31c0

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ccb2effe31c0