1194182 - Implement switch-to-tab for add-on toast notification

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P5)

Description

[u421692]

1. Have add-on manager tab opened
2. Open AMO and install an add-on
3. When the toast notification(installation complete) is displayed, tap on add-ons button from toast

Actual result:
add-on manager is opened in a new tab

Expected result:
It should switch to the tab where add-on manager is already opened

Comment 1

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

Attachment: update-selectoropentab v0.1

This patch refactors the BrowserApp.selectOrOpenTab:
* Renames to selectOrAddTab since that mirrors the underlying methods
* Adds a param argument that is passed to addTab, if called. Can be null.

Comment 3

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

Attachment: selectoradd-addons v0.1

Switches the two places where we open "about:addons" from using addTab to selectOrAddTab. Also removes the hash from the toast. (bug 1173893)

Comment 4

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

Attachment: toast-addon-telemetry v0.1

UI Telemetry for the toast

Comment 5

[:Margaret Leibovic]

Comment on attachment 8672114 [details] [diff] [review]
selectoradd-addons v0.1

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

I guess this is fine, so I don't want to be stop energy about it, but I think we should clean up this code and get rid of unused options and code paths so that we can reduce the number of parameters needed here.

::: mobile/android/chrome/content/browser.js
@@ +1330,5 @@
>     * Otherwise, a new tab is opened with the given URL.
>     *
>     * @param aURL URL to open
> +   * @param aParam Options used if a tab is created
> +   * @param aFlags Options for the search. Currently supports:

This is confusing... right now the only supported flag is "startsWith", and the only consumer of this method sets that flag to true, so maybe we should get rid of this option and just set this as the default behavior. This would mean we could also get rid of the options method (and an used code path) in getTabWithURL.

Comment 6

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

(In reply to :Margaret Leibovic from comment #5)
> Comment on attachment 8672114 [details] [diff] [review]
> selectoradd-addons v0.1
> 
> Review of attachment 8672114 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I guess this is fine, so I don't want to be stop energy about it, but I
> think we should clean up this code and get rid of unused options and code
> paths so that we can reduce the number of parameters needed here.

DownloadNotifications.jsm uses { startsWith: true } , but the two browser.js uses of selectOrAdd do not use the flag. We should make sure that all consumers could handle one case or the other before changing them.

I agree that simplifying would be great.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/49877ebcdca5
https://hg.mozilla.org/integration/fx-team/rev/dca9314ec360
https://hg.mozilla.org/integration/fx-team/rev/5d9a9f5ca4d4

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/49877ebcdca5
https://hg.mozilla.org/mozilla-central/rev/dca9314ec360
https://hg.mozilla.org/mozilla-central/rev/5d9a9f5ca4d4