Opt-out dialog has a button for "disable", but actually the addon is removed

RESOLVED FIXED in 4.0.0.1

Status

Calendar
Calendar Views
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Fallen, Assigned: MakeMyDay)

Tracking

Trunk
4.0.0.1
Dependency tree / graph

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
STR:

In a new profile, wait for the opt-out bar.
Click the "Disable" button.
Restart

Results:

The addon is completely removed

Expected:

The addon is just disabled, or the button should say "Remove". If we don't have strings ourselves for this, we can use the addon manager strings.

MakeMyDay, can you take a look?
Flags: needinfo?(makemyday)

Comment 1

2 years ago
I think just disabling is preferable.
http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/messenger-overlay-sidebar.js#216
(Reporter)

Comment 2

2 years ago
See bug 1130852 comment 25 to 27, comment 33, comment 36. I think this just got lost in the discussion on the amount of prefs. MakeMyDay prefers uninstall, some others prefer disable. MakeMyday, maybe you can make your case here again :)

Comment 3

2 years ago
The user can still manually uninstall, right? I'd also support simply disabling.
(Assignee)

Comment 4

2 years ago
Created attachment 8601710 [details] [diff] [review]
DisableInsteadOfRemoveLightning-V1.diff

As discussed on the meeting, we keep the strings and change the behaviour to disabling instead of removing. This patch takes care.
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Flags: needinfo?(makemyday)
Attachment #8601710 - Flags: review?(philipp)
(Assignee)

Comment 5

2 years ago
One more thing that came to my mind: does it make any difference for the AMO statistic whether we disable or remove?
(Reporter)

Comment 6

2 years ago
Comment on attachment 8601710 [details] [diff] [review]
DisableInsteadOfRemoveLightning-V1.diff

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

r=philipp with this comment considered:

::: calendar/lightning/content/messenger-overlay-sidebar.js
@@ +218,3 @@
>      };
>      let cbUndoRemoveLightning = function (aAddon) {
> +        aAddon.cancelUninstall();

Shouldn't the undo action be aAddon.userDisabled = false?
Attachment #8601710 - Flags: review?(philipp)
Attachment #8601710 - Flags: review+
Attachment #8601710 - Flags: approval-calendar-beta+
Attachment #8601710 - Flags: approval-calendar-aurora+
(Reporter)

Comment 7

2 years ago
I don't think disabled addons contribute to ADU, they are not active users :)
(Assignee)

Comment 8

2 years ago
Created attachment 8602251 [details] [diff] [review]
DisableInsteadOfRemoveLightning-V2.diff

> Shouldn't the undo action be aAddon.userDisabled = false?

Yes, you're right. Updated patch with comment considered.
Attachment #8601710 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Reporter)

Updated

2 years ago
Attachment #8602251 - Flags: review+
Attachment #8602251 - Flags: approval-calendar-beta+
Attachment #8602251 - Flags: approval-calendar-aurora+
(Reporter)

Comment 9

2 years ago
Pushed to comm-central changeset 099ffdaa0416
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.2
(Reporter)

Comment 10

2 years ago
Backported to releases/comm-aurora changeset 89722d50d1f1
Target Milestone: 4.2 → 4.1
(Reporter)

Comment 11

2 years ago
Backported to releases/comm-beta changeset d378aa365a2d
Target Milestone: 4.1 → 4.0
You need to log in before you can comment on or make changes to this bug.