Notify user when an add-on is already installed

RESOLVED DUPLICATE of bug 1245571

Status

()

Firefox for Android
Add-on Manager
P5
enhancement
RESOLVED DUPLICATE of bug 1245571
3 years ago
5 months ago

People

(Reporter: u421692, Assigned: miketaylr)

Tracking

Trunk
All
Android
Points:
---

Firefox Tracking Flags

(firefox43 affected, fennec+)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
1. Go to AMO and install an add-on
2. After the installation was finished, tap again on "Add to Firefox" button
3. Tap on Install button

Actual result:
No notification is displayed, so if the user forgets that has already installed, there is no feedback(poor UX)

Expected result:
User should be notified if the add-on is already installed

Updated

3 years ago
tracking-fennec: --- → ?
Assignee: nobody → margaret.leibovic
tracking-fennec: ? → +

Comment 1

3 years ago
So, right now we don't show a notification when an already installed add-on is re-installed:
http://hg.mozilla.org/mozilla-central/annotate/29b2df16e961/mobile/android/chrome/content/browser.js#l6335

So, maybe we should add a toast message here to explain that the add-on was already installed? However, it's not like we're not installing something new - it's just replacing what was already installed.

antlam, do you have ideas about what to do here? I feel like the best solution might be to just remove this check that prevents us from showing the add-on installed toast when it's already an existing add-on.
Flags: needinfo?(alam)
Is there anyway that we can show "Already installed" in place of the green "+ Add to Firefox" button IF the user already has this add-on? Ideally, I think a lot of that feedback should be on the button UI (like our sync/ fxa sign up flow)

If not, then yes a "Add-on already installed" toast is probably the best starting point here.
Flags: needinfo?(alam) → needinfo?(margaret.leibovic)

Comment 3

3 years ago
(In reply to Anthony Lam (:antlam) from comment #2)
> Is there anyway that we can show "Already installed" in place of the green
> "+ Add to Firefox" button IF the user already has this add-on? Ideally, I
> think a lot of that feedback should be on the button UI (like our sync/ fxa
> sign up flow)

No, this wouldn't be possible without web content somehow being able to know which add-ons you have installed. While it is theoretically possible we could do some special white-listing for AMO, I don't know that it's worth the effort (and it would require help from the AMO team).

> If not, then yes a "Add-on already installed" toast is probably the best
> starting point here.

So, yeah, I think we should do this :)
Flags: needinfo?(margaret.leibovic)
(In reply to :Margaret Leibovic from comment #1)
> So, right now we don't show a notification when an already installed add-on
> is re-installed:
> http://hg.mozilla.org/mozilla-central/annotate/29b2df16e961/mobile/android/
> chrome/content/browser.js#l6335
> 
> So, maybe we should add a toast message here to explain that the add-on was
> already installed? However, it's not like we're not installing something new
> - it's just replacing what was already installed.
> 
> antlam, do you have ideas about what to do here? I feel like the best
> solution might be to just remove this check that prevents us from showing
> the add-on installed toast when it's already an existing add-on.

It seems like for add-ons that "do things" like open a resource page or add a menu item get to "do" them for each time that you install, until you re-install the browser. At least that's how I understand Bug 1197535.

So in addition to showing a toast (which I think is a good thing), we should prevent re-installs if possible. I can't think of a good reason why I would want to install an addon more than once.

Comment 5

2 years ago
I haven't had time to get back to this bug... Mike, you're welcome to take this from me if you're interested in making a patch! :)
Flags: needinfo?(miket)
Cool, let me steal it then. I'll ping you if I get stuck.
Assignee: margaret.leibovic → miket
Flags: needinfo?(miket)
Hmm, it seems Desktop is happy to let me install the same add-on multiple times. The one difference is Bug 1197535 -- if an Add-on opens a resource page, for example, it only happens once in Desktop (rather than each time for each re-install). 

Margaret, do you think we should continue w/ the plan to toast and prevent re-installs? Or should we follow Desktop (and fix Bug 1197535)? I'm less sure knowing that Desktop allows it.
Flags: needinfo?(margaret.leibovic)

Comment 8

2 years ago
(In reply to Mike Taylor [:miketaylr] from comment #7)
> Hmm, it seems Desktop is happy to let me install the same add-on multiple
> times. The one difference is Bug 1197535 -- if an Add-on opens a resource
> page, for example, it only happens once in Desktop (rather than each time
> for each re-install). 
> 
> Margaret, do you think we should continue w/ the plan to toast and prevent
> re-installs? Or should we follow Desktop (and fix Bug 1197535)? I'm less
> sure knowing that Desktop allows it.

Maybe I wasn't clear before, but I believe that Fennec does also let you install the same add-on multiple times, it just won't necessarily have much of an effect if it's replacing an existing add-on. Perhaps the suggested string for the toast message should be revised to reflect that.

Since we share our core add-on manager logic with desktop, I think the best solution would be to do the same thing, and continue to allow an add-on to be re-installed.

The problem in bug 1197535 sounds like the uninstall logic from the buggy version of the add-on wasn't called, since it would be up to the add-on to clean up after itself by removing its own pieces of the UI.

I could be wrong, so I would want to make a test add-on to see what happens.
Flags: needinfo?(margaret.leibovic)
OK, so plan is to detect if we're installing an add-on that's already installed and then send a toast to the user so they're less confused. We're re-installing, since that's what Desktop does (AFAIU).

Brainstorming some strings here...some of questionable quality:

1) "Add-on already installed" (from Comment #2)
2) "Add-on re-installed."
3) "Existing add-on installed."
4) "Already installed add-on installed."
5) "Existing add-on replaced."

#1 makes it sound like installation was stopped somehow. I'm on the fence between #2 and #3. Any other suggestions antlam?
Flags: needinfo?(alam)
I'm gonna stick with "Add-on already installed" in a Snackbar for now :)

Thanks Mike!
Flags: needinfo?(alam)
Created attachment 8706648 [details] [diff] [review]
Notify user when existing add-on is installed.
(In reply to :Margaret Leibovic from comment #8)
> The problem in bug 1197535 sounds like the uninstall logic from the buggy
> version of the add-on wasn't called, since it would be up to the add-on to
> clean up after itself by removing its own pieces of the UI.
> 
> I could be wrong, so I would want to make a test add-on to see what happens.

Just to clarify, I believe your test add-on comment is referring to bug 1197535, right Margaret?

Any suggestion for how to test this patch? Poking around the tree I don't see any about:addons tests for Fennec...
Flags: needinfo?(margaret.leibovic)

Comment 13

2 years ago
(In reply to Mike Taylor [:miketaylr] from comment #12)
> (In reply to :Margaret Leibovic from comment #8)
> > The problem in bug 1197535 sounds like the uninstall logic from the buggy
> > version of the add-on wasn't called, since it would be up to the add-on to
> > clean up after itself by removing its own pieces of the UI.
> > 
> > I could be wrong, so I would want to make a test add-on to see what happens.
> 
> Just to clarify, I believe your test add-on comment is referring to bug
> 1197535, right Margaret?
> 
> Any suggestion for how to test this patch? Poking around the tree I don't
> see any about:addons tests for Fennec...

Our Android add-on automated test situation is really abysmal... I think most of this code was written in a mad dash as we re-wrote Fennec Native, and we didn't have good JS test support at the time. We mostly rely on the toolkit add-on tests, but that doesn't cover Android UI interactions, so we really should have our own tests.

I think the best way to test this would be to write some mochitests:
https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#mochitest_.28plain_and_chrome.29

There are some example tests you can use for inspiration here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/chrome/

We should also file more bugs for writing tests for add-ons on Android. This would be a great place to contribute!
Flags: needinfo?(margaret.leibovic)

Comment 14

2 years ago
The API is being added in bug 1245571, AMO changes being landed in https://github.com/mozilla/olympia/issues/1157.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1245571
You need to log in before you can comment on or make changes to this bug.