Closed Bug 706292 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: mossop, Assigned: MattN)

Details

Attachments

(1 file, 2 obsolete files)

Since it follows different settings there is no point in showing this UI to the users. See bug 694068 comment 7
Attached patch WIP without tests (obsolete) — Splinter Review
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.
Assignee: nobody → mnoorenberghe+bmo
Status: NEW → ASSIGNED
Attachment #616208 - Flags: feedback?(bmcbride)
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).
Attachment #616208 - Flags: feedback?(bmcbride) → feedback+
Added pref observer and hotfixID property to AddonManager and AddonManagerInternal like the other prefs.
Attachment #616208 - Attachment is obsolete: true
Attachment #624199 - Flags: review?(bmcbride)
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.
Attachment #624199 - Flags: review?(bmcbride) → review-
Attachment #624199 - Attachment is obsolete: true
Attachment #624311 - Flags: review?(bmcbride)
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 :)
Attachment #624311 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/ccb2effe31c0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.