Closed Bug 714070 Opened 13 years ago Closed 10 years ago

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


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



(firefox34 verified)

Firefox 34
Tracking Status
firefox34 --- verified


(Reporter: martijn.martijn, Assigned: rahul.parsani, Mentored)




(Whiteboard: [lang=javascript][good next bug])


(2 files, 1 obsolete file)

Steps to reproduce:
- Open
- 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
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.
Priority: -- → P4
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).
Whiteboard: [mentor=wesj]
Whiteboard: [mentor=wesj] → [mentor=wesj][lang=java]
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?
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).
Whiteboard: [mentor=wesj][lang=java] → [mentor=wesj][lang=javascript]
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!
Hi this is my first bug. Can anyone get me started?
Sure. Like I said above, step 1 is building. There are some fairly good instructions on that here:

Feel free to comment here, by email, or on (#mobile) if you need help.
See also bug 884075 where this was implemented.
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?
Whiteboard: [mentor=wesj][lang=javascript] → [mentor=wesj][lang=javascript][good next bug]
Mentor: wjohnston
Whiteboard: [mentor=wesj][lang=javascript][good next bug] → [lang=javascript][good next bug]
Attached patch 714070.patch (obsolete) — Splinter Review
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.
Attachment #8472733 - Flags: review+
Attachment #8472733 - Flags: review+ → review?(wjohnston)
Comment on attachment 8472733 [details] [diff] [review]

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/
@@ +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 @@
>, "short");
>    },
> +  
> +  openAddonManager: function() {
> +    sendMessageToJava({ type: "AddonsManager:Open" });

You should be able to just use BrowserApp.addTab() here, like this:

(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/
@@ +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?
Attachment #8472733 - Flags: review?(wjohnston) → review-
Attached image 714070.png
Screenshot for the string/icon interaction.
Attached patch 714070.patchSplinter Review
Removed the sendMessageToJava call. 

BrowserApp.addTab only needed the parentId param for the back button to work correctly.
Attachment #8472733 - Attachment is obsolete: true
Attachment #8473393 - Flags: review?(wjohnston)
I haven't changed the string for the ButtonToast. Based on the screenshot, let me know what it should be.
Comment on attachment 8473393 [details] [diff] [review]

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

Nice. Your string is mfinkle approved too, so we can use it :)
Attachment #8473393 - Flags: review?(wjohnston) → review+
Can we get a Try run first please? :)
Keywords: checkin-needed
Assignee: nobody → rahul.parsani
Keywords: checkin-needed
Keywords: checkin-needed
Whiteboard: [lang=javascript][good next bug] → [lang=javascript][good next bug][fixed-in-fx-team]
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [lang=javascript][good next bug][fixed-in-fx-team] → [lang=javascript][good next bug]
Target Milestone: --- → Firefox 34
Verified as fixed in
Build: Firefox for Android 34.0a2 (2014-09-17)
Device: Asus Transformer Pad TF300T (Android 4.2.1)
Depends on: 1173893
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.