Closed Bug 1159698 Opened 5 years ago Closed 5 years ago

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

Categories

(Calendar :: Calendar Views, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
4.0.0.1

People

(Reporter: Fallen, Assigned: MakeMyDay)

References

Details

Attachments

(1 file, 1 obsolete file)

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)
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 :)
The user can still manually uninstall, right? I'd also support simply disabling.
Attached patch DisableInsteadOfRemoveLightning-V1.diff (obsolete) β€” β€” Splinter Review
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)
One more thing that came to my mind: does it make any difference for the AMO statistic whether we disable or remove?
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+
I don't think disabled addons contribute to ADU, they are not active users :)
> Shouldn't the undo action be aAddon.userDisabled = false?

Yes, you're right. Updated patch with comment considered.
Attachment #8601710 - Attachment is obsolete: true
Keywords: checkin-needed
Attachment #8602251 - Flags: review+
Attachment #8602251 - Flags: approval-calendar-beta+
Attachment #8602251 - Flags: approval-calendar-aurora+
Pushed to comm-central changeset 099ffdaa0416
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.2
Backported to releases/comm-aurora changeset 89722d50d1f1
Target Milestone: 4.2 → 4.1
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.