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

VERIFIED FIXED in Firefox 34

Status

()

defect
P4
normal
VERIFIED FIXED
8 years ago
4 years ago

People

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

Tracking

Trunk
Firefox 34
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox34 verified)

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

8 years ago
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
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]

Comment 3

6 years ago
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]

Comment 5

6 years ago
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

6 years ago
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:

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

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

Comment 8

6 years ago
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]
Assignee

Comment 10

5 years ago
Posted 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+
Assignee

Updated

5 years ago
Attachment #8472733 - Flags: review+ → review?(wjohnston)
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?
Attachment #8472733 - Flags: review?(wjohnston) → review-
Assignee

Comment 12

5 years ago
Posted image 714070.png
Screenshot for the string/icon interaction.
Assignee

Comment 13

5 years ago
Posted 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)
Assignee

Comment 14

5 years ago
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]
714070.patch

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
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/32afd8ecf08e
Keywords: checkin-needed
Whiteboard: [lang=javascript][good next bug] → [lang=javascript][good next bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/32afd8ecf08e
Status: NEW → RESOLVED
Closed: 5 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)
Status: RESOLVED → VERIFIED

Updated

4 years ago
Depends on: 1173893
You need to log in before you can comment on or make changes to this bug.