714070 - Tapping on the "Installation completed" toaster notification should open up the Add-ons manager

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Open http://people.mozilla.org/~mfinkle/addons/
- Tap on "Install" for one of the Add-ons
- Tap on "Allow" for the prevention doorhanger notification
- Tap on "Install" when the install dialog appears
- When the "Installation completed" toaster notification appears, tap on it

Expected result:
- Add-ons manager page appears

Actual result:
- Nothing happens

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Standard Android toasts don't really support a "click and do something" behavior. We'd need a custom toast for that, and I don't know that we really need it.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

We have super toasts now, making wesj the mentor on this old bug (feel free to bounce it back to me if you don't want to mentor this).

Comment 3

[basscase]

Hi I'm looking to become involved in fennec and I'm thinking of working on this bug as my first task. I learned java and android in university and would like to use what I learned for a project bigger than personal apps. I see that wesj is the mentor on this, how to you feel about me doing this as my first bug for fennec?

Comment 4

[Wesley Johnston (:wesj)]

Sure! It should be a fairly simple fix, but most (all?) of the changes should be in javascript (the mentor notes were wrong, I just fixed them). Are you ok with that?

Regardless, step 1 and the hardest step here is probably just to get Fennec building and installing on a device or the emulator (but preferably a device).

Comment 5

[basscase]

Thanks I might come back to this bug when I've completed my first. I think it would be better if my first bug uses java which is the language i'm most comfortable with. Once I know my way around fennec a bit more I I'll consider working on bugs with languages less familiar to me. I'm looking at a few others from the get involved page but if you know of any others that require java and you think would be suitable for a first timer let me know, I look forward to participating!

Comment 6

[Michael]

Hi this is my first bug. Can anyone get me started?

Comment 7

[Wesley Johnston (:wesj)]

Sure. Like I said above, step 1 is building. There are some fairly good instructions on that here:

https://wiki.mozilla.org/Mobile/Fennec/Android

Feel free to comment here, by email, or on irc.mozilla.org (#mobile) if you need help.

Comment 8

[Martijn Wargers (dead)]

See also bug 884075 where this was implemented.

Comment 9

[Mike Hoye [:mhoye]]

M(In reply to Michael from comment #6)
> Hi this is my first bug. Can anyone get me started?

Hi, Michael - are you still working on this?

Comment 10

[Rahul Parsani]

Attachment: 714070.patch

Added a new native event. There is a new string that is the label for the ButtonToast. The button toast is only generated when a addon is installed. Clicking on it should take you to the addon tab. Let me know if this can be improved.

Comment 11

[Wesley Johnston (:wesj)]

Comment on attachment 8472733 [details] [diff] [review]
714070.patch

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

I think we can do this all in JS. I also think we might need to think about the string/icon a bit, but I don't mind landing and iterating on that later.

::: mobile/android/base/GeckoApp.java
@@ +521,5 @@
>              GeckoAccessibility.updateAccessibilitySettings(this);
>  
> +        } else if ("AddonsManager:Open".equals(event)) {
> +            Tabs.getInstance().loadUrlInTab(AboutPages.ADDONS);
> +            

extra whitespace here, but I don't think you'll need this...

::: mobile/android/chrome/content/browser.js
@@ +6116,5 @@
>      NativeWindow.toast.show(msg, "short");
>    },
> +  
> +  openAddonManager: function() {
> +    sendMessageToJava({ type: "AddonsManager:Open" });

You should be able to just use BrowserApp.addTab() here, like this:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3012

(you probably don't need all those options). That will get rid of all this message passing for you. You can open about:addons, or you could open something like about:addons#myAddingID if we have it. I doubt the add-on manager does anything with that yet (but we can do that in a separate bug).

::: mobile/android/locales/en-US/chrome/browser.properties
@@ +11,5 @@
> +alertAddonsInstalledNoRestart.message=Installation complete
> +
> +# LOCALIZATION NOTE (alertAddonsInstalledNoRestart.action2): Ideally, this string is short (it's a
> +# button label) and upper-case, to match Google and Android's convention.
> +alertAddonsInstalledNoRestart.action2=ADD-ONS

Hmm... We probably need some UX feedback on this string/icon interaction. I think this should be a verb. "OPEN" maybe? "SETUP"? Can you post a screenshot for them at least?

Comment 12

[Rahul Parsani]

Attachment: 714070.png

Screenshot for the string/icon interaction.

Comment 13

[Rahul Parsani]

Attachment: 714070.patch

Removed the sendMessageToJava call. 

BrowserApp.addTab only needed the parentId param for the back button to work correctly.

Comment 14

[Rahul Parsani]

I haven't changed the string for the ButtonToast. Based on the screenshot, let me know what it should be.

Comment 15

[Wesley Johnston (:wesj)]

Comment on attachment 8473393 [details] [diff] [review]
714070.patch

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

Nice. Your string is mfinkle approved too, so we can use it :)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Can we get a Try run first please? :)

Comment 17

[Wesley Johnston (:wesj)]

https://tbpl.mozilla.org/?tree=Try&rev=a71dd04de579

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/32afd8ecf08e

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/32afd8ecf08e

Comment 20

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in
Build: Firefox for Android 34.0a2 (2014-09-17)
Device: Asus Transformer Pad TF300T (Android 4.2.1)