1418466 - Add Connect Another Device button to relevant Sync UI

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P1)

Description

[Ryan Feeley [:rfeeley]]

This is a bug to update sync-related UI for single-device syncers to make it easier for them to connect another device.

1. Remove the the lengthy "Download Firefox for Android or Firefox for iOS and connect them to your Firefox Account." and "No synced tabs… yet" text from the sidebar and menu panel
2. Replace them both with a "Connect Another Device" button.
3. In the page action menu, add a "Connect Another Device…" option above the "Learn About Sending Tabs…" marketing link.

The URLs should be:
https://accounts.firefox.com/connect_another_device?service=sync&context=fx_desktop_v3&entrypoint={entrypoint}

Entrypoint in menu panel = synced-tabs
Entrypoint in sidebar = tabs-sidebar
Entrypoint in page action menu = sendtab

Comment 1

[Edouard Oger [:eoger]]

Attachment: Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

Review commit: https://reviewboard.mozilla.org/r/200914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200914/

Comment 2

[Ryan Feeley [:rfeeley]]

Attachment: context-menus-too.png

Almost forgot the context menus (I'm not sure what the entrypoint would be from those). We'd prefer the CAD link to be above Learn link, but if it's not practical, below is fine.

Comment 3

[Edouard Oger [:eoger]]

Yup it's included in the patch, "sendtab" will be used too.

Comment 4

[Ryan Feeley [:rfeeley]]

███████████████████████████████████████
███████████████████████████████████████
█░░██░░████▀▄▄▄▀███████████████░░░░░░▀█
█░░██░░████░███░██████████▀▄▄▀█░░███░░█
█░░██░░████░███░██████████░██░█░░░░░░▄█
█░░██░░████░███░██████████░██░█░░▄░░▀▀█
█░░██░░████░███░██████████░██░█▄▄██▄▄▄█
█░░██░░████░███░█▀▄▄▄▀████░██░█▀░░░░░▀█
█░░██░░████░███░▀░███░▄▄▀█░██░█░░███░░█
█░░██░░████░██▀▄▄▄▀██░██░▄███░█░░███░░█
█░░██░░███▀░▄▄████░██░██░████░█▄░░░░░▄█
█░░██░░██░▄███▀▀░▄░██░██░████░██▀▀▀▀▀██
█░░██░░█░██▀▀░▄███░▀▀░██░████░█░░▄▄▄░░█
█░░██░░█░██░▀██████▄▄█▀▀▄████░█░░██████
█░░██░░█░███▄▄▄██████████████░█░░▀▀▀░░█
█░░██░░██░▀██████████████████░██▄▄▄▄▄██
█░░██░░███▄░▀████████████████░█░░█▀░░▄█
█░░██░░█████▄░▀███████████▀▄▄██░░░░▄███
█░░▀▀░░████████░█████████░█████░░▄░░▀██
██▄░░▄█████████░█████████░█████▄▄██▄▄▄█
███████████████▄█████████▄█████████████
███████████████████████████████████████

Comment 5

[Shane Tomlinson [:stomlinson]]

To make this work as expected, we are going to need to make content-server changes. 

As things stand today, the content server decides whether to send a user to 
/sms or /connect_another_device on /verify_email and other /complete_* screens.

/connect_another_device asks the user to install Firefox on their mobile device
but only shows the user the buttons to the app stores whereas /sms also allows
the user to send themselves an SMS with an install link.

By sending users directly to /connect_another_device, eligible users will
never be sent to /sms.

On the content server, we'll need to update the /connect_another_device endpoint
so that it makes the decision whether to stay on /connect_another_device or
send the user to /sms. This shouldn't be difficult to do, but we should make
this update before this patch works its way to beta/release or else
we are going to start wondering why the number of users going to /sms
drops in eligible countries.

Comment 6

[Shane Tomlinson [:stomlinson]]

Ref https://github.com/mozilla/fxa-content-server/issues/5737

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

https://reviewboard.mozilla.org/r/200914/#review206950

::: browser/base/content/browser-sync.js:421
(Diff revision 1)
>      separator.classList.add("sync-menuitem");
>      fragment.appendChild(separator);
>  
> -    const actionItem = createDeviceNodeFn(null, actionLabel, null);
> -    actionItem.addEventListener("command", actionCommand, true);
> +    for (let i = 0; i < actions.length; i += 2) {
> +      const label = actions[i];
> +      const command = actions[i + 1];

This is pretty ugly. Can you make actions an object ({label: command, another_label: another_command}), a Map, an array of arrays or something like that?

Comment 8

[Edouard Oger [:eoger]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200914/diff/1-2/

Comment 9

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

https://reviewboard.mozilla.org/r/200914/#review208728

::: browser/app/profile/firefox.js:1425
(Diff revision 2)
>  pref("identity.fxaccounts.settings.uri", "https://accounts.firefox.com/settings?service=sync&context=fx_desktop_v3");
>  
>  // The URL of the FxA device manager page
>  pref("identity.fxaccounts.settings.devices.uri", "https://accounts.firefox.com/settings/clients?service=sync&context=fx_desktop_v3");
>  
> +// The URL to a page that explains how to connect another device to Sync.

Can you please move this next to the 2 other "mobile promo" strings - I expect bug 1420701 might either remove those other ones, but having them together makes some sense to me (and the comments can clarify that about:prefs does things differently in the meantime)

Comment 10

[Edouard Oger [:eoger]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200914/diff/2-3/

Comment 11

[Edouard Oger [:eoger]]

Thanks you Mark.
NI? myself to land this once bug 1417520 has landed.

Comment 12

[Edouard Oger [:eoger]]

Woops, forgot that I was awaiting on Dao's review, nevermind :)

Comment 13

[Shane Tomlinson [:stomlinson]]

content-server PR to redirect from /connect_another_device to eligible users opened: 
https://github.com/mozilla/fxa-content-server/pull/5766

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

https://reviewboard.mozilla.org/r/200914/#review209762

::: browser/base/content/browser-sync.js:276
(Diff revision 3)
>    },
>  
> +  openConnectAnotherDevice(entryPoint) {
> +    let url = new URL(Services.prefs.getCharPref("identity.fxaccounts.remote.connectdevice.uri"));
> +    url.searchParams.append("entrypoint", entryPoint)
> +    switchToTabHavingURI(url.href, true);

This should use openUILinkIn, not switchToTabHavingURI.

Comment 15

[Edouard Oger [:eoger]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200914/diff/3-4/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/932a77d8fbae
Add Connect Another Device button to relevant Sync UI. r=dao,markh

Comment 17

[Noemi Erli[:noemi_erli]]

Backed out changeset 932a77d8fbae (bug 1418466) for ES failures in /builds/worker/checkouts/gecko/browser/base/content/browser-sync.js r=backout on a CLOSED TREE

https://hg.mozilla.org/integration/autoland/rev/ec14d683fb71846ec945e81902655bf52fda9568

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=932a77d8fbaec3422da1104c060a40b490917a79&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Comment 18

[Edouard Oger [:eoger]]

Comment on attachment 8929637 [details]
Bug 1418466 - Add Connect Another Device button to relevant Sync UI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200914/diff/4-5/

Comment 19

[Edouard Oger [:eoger]]

Thank you!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/469b5d1b8067
Add Connect Another Device button to relevant Sync UI. r=dao,markh

Comment 21

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/469b5d1b8067