1213490 - Correct UX of top level share plane when < 2 devices are synced

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

via https://bugzilla.mozilla.org/show_bug.cgi?id=1140048#c58:

Wanted to call out the various scenarios/flows here:

Not signed in:
 - Proceed to FxA sign up flow

User is signed in, but has no other connected devices:
 - Display toast with messaging
 - - "Send this tab to another connected device" 

 - At a later point, possibly experiment with using our "in-product promo sheet" UX (bug to-be-filed for this V2 polish)

User is signed in, has 1 other connected device:
 - Just send the tab, confirmation UI will follow (being handled in bug 1147653)

User is signed in, has 2+ other connected device:
 - Show device list, then send the tab to selected device, followed by confirmation UI.

For context, this button sends the current tab to a Firefox Sync connected device. But, we need to display a message for users that don't have another device to send to. 

Essentially, we want the users to understand that they need to sign in on _another_ device to fully benefit from this feature.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #0)
> User is signed in, but has no other connected devices:
>  - Display toast with messaging
>  - - "Send this tab to another connected device" 

I don't really like this String – I think it needs to focus on connecting another device so:

"Connect another device to your account to send this tab"

Anthony, what do you think?

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1213490 - Move send to device handling code to a separate method. r=nalexander

Bug 1213490 - Move send to device handling code to a separate method. r=nalexander

This makes space to do more in the upcoming patches.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1213490 - Handle shareplane when device list cannot be shown. r=nalexander

Bug 1213490 - Handle shareplane when device list cannot be shown. r=nalexander

The handled cases are:
  * The user is not signed in (sync setup is shown)
  * The user is signed in but has no other synced devices (toast is shown)

In the future, if there is one other synced device, we should send directly to
that device (bug 1217167).

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8677093 [details]
MozReview Request: Bug 1213490 - Handle shareplane when device list cannot be shown. r=nalexander

https://reviewboard.mozilla.org/r/22881/#review20405

I believe the threading is wrong because the initial option select will happen on the UI thread, but I trust you to get that right :)

::: mobile/android/base/BrowserApp.java:3549
(Diff revision 1)
> +     *   * TODO (bug 1217167) Signed in but 1 other device: send tab & display confirmation

nit: with exactly 1 other device...

::: mobile/android/base/BrowserApp.java:3552
(Diff revision 1)
>      private void handleSendToDevice() {

Comment on the thread restrictions, please.  (This does blocking synchronous IO.)

There's an `AccountLoader`; is it feasible to use it here?

I've been pondering extracting a `Service` that would maintain the Firefox Account state over time, caching this information as much as possible.  Something to think about.

::: mobile/android/base/BrowserApp.java:3583
(Diff revision 1)
> +    private int getOtherSyncClientCount() {

Move this into TabsAccessor?

::: mobile/android/base/locales/en-US/android_strings.dtd:152
(Diff revision 1)
> +<!ENTITY menu_no_synced_devices "Send this tab to another connected device">

This doesn't say what to do.  "Connect another device to send this tab to"?  That's awkward.  There's no immediate action the user can take on this device to achieve the goal -- I wonder if we should send them to accounts.firefox.com and give some instruction on how to connect another device, or offer a QR code/email/...

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) from comment #1)
> (In reply to Michael Comella (:mcomella) from comment #0)
> > User is signed in, but has no other connected devices:
> >  - Display toast with messaging
> >  - - "Send this tab to another connected device" 
> 
> I don't really like this String – I think it needs to focus on connecting
> another device so:
> 
> "Connect another device to your account to send this tab"
> 
> Anthony, what do you think?

I prefer not having multiple "to" in the same sentence because it sounds more complicated of a task.

I also wanted to include "send" in the beginning to give it more prominence. Especially if people are just scanning the sentence, this "action" is very clear as what they want to do as opposed to what needs to be set up.

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8677092 [details]
MozReview Request: Bug 1213490 - Move send to device handling code to a separate method. r=nalexander

https://reviewboard.mozilla.org/r/22879/#review20401

Gah!  Sorry, I started this yesterday, and must not have hit publish.

::: mobile/android/base/BrowserApp.java:3539
(Diff revision 1)
> +        final Tab tab = Tabs.getInstance().getSelectedTab();

I think it makes more sense to extract the tab URL at the call site.  (It's weird to look at the selected tab without context.)

Also, consider returning a boolean indicating whether we tried to send the tab.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/22879/#review20401

> I think it makes more sense to extract the tab URL at the call site.  (It's weird to look at the selected tab without context.)
> 
> Also, consider returning a boolean indicating whether we tried to send the tab.

> I think it makes more sense to extract the tab URL at the call site.  (It's weird to look at the selected tab without context.)

Done.

> Also, consider returning a boolean indicating whether we tried to send the tab.

We can do this if we ever need the return value – better to not leave in untested code.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/22881/#review20405

Corrected by putting calling `ThreadUtils.postToBackgroundThread` before calling `handleSendToDevice` and adding the `@WorkerThread` annotation to `handleSendToDevice`.

> nit: with exactly 1 other device...

Going to remove this line because bug 1217167 is wontfixed.

> Comment on the thread restrictions, please.  (This does blocking synchronous IO.)
> 
> There's an `AccountLoader`; is it feasible to use it here?
> 
> I've been pondering extracting a `Service` that would maintain the Firefox Account state over time, caching this information as much as possible.  Something to think about.

> Comment on the thread restrictions, please.  (This does blocking synchronous IO.)

Done via @WorkerThread annotation (which don't work – see bug 1219067).

> There's an AccountLoader; is it feasible to use it here?

Discussed on irc – it can be done but it's probably not worth the work, especially if we move towards the `Service` approach. We can use the `AccountLoader` but in order to prevent extra work each time the Account state is updated, we need to have a single `AccountLoader` in one `LoaderCallbacks` instance. The logic should all move to `BrowserApp` and the `LoaderCallbacks` should update the `RemoteTabsPanel` as well as the send to device state. However, this creates a dependency between the Fragment and the main Activity so it'd need to be handled carefully.

> This doesn't say what to do.  "Connect another device to send this tab to"?  That's awkward.  There's no immediate action the user can take on this device to achieve the goal -- I wonder if we should send them to accounts.firefox.com and give some instruction on how to connect another device, or offer a QR code/email/...

Let's handle this in bug 1217174 – I flagged for triage and am hoping to land in the same version, but it may be dependent on SUMO.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/607576699f93afbb0f30e248511c05f441e4148a
Bug 1213490 - Move send to device handling code to a separate method. r=nalexander

https://hg.mozilla.org/integration/fx-team/rev/e0fc8e81cc7013751be22a27b538ddf1ffabcbc3
Bug 1213490 - Handle shareplane when device list cannot be shown. r=nalexander

https://hg.mozilla.org/integration/fx-team/rev/81f5d579fb2bbef17eeb2a9a80c16d05fc9e399d
Bug 1213490 - review: Move BrowserApp.getOtherSyncClientCount -> TabsAccessor.getRemoteClientCount. r=me

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/607576699f93
https://hg.mozilla.org/mozilla-central/rev/e0fc8e81cc70
https://hg.mozilla.org/mozilla-central/rev/81f5d579fb2b

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Backing out to target 45 – see bug 1140048 comment 74.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Backout of 44, as per comment 11: https://hg.mozilla.org/releases/mozilla-aurora/rev/c48421a260c1

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/3c8c36bf9e0d93ff5a145b6c22fd035a4720ec3a
Bug 1213490 - Backout 3 changesets to backout top level shareplane (bug 1140048).

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

re backout: see bug 1140048 comment 83. Since we backed out the bug 1140048 that added this bug, this does not affect any versions until bug 1140048 lands again.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Since bug 1140048 is backed out, not actively working on this.

Comment 16

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.