Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Add a "Sync Preferences" button to the account ready page [desktop sync][opened via a new tab] #3079

Closed
mhammond opened this issue Sep 18, 2015 · 27 comments · Fixed by #3341
Closed
Assignees

Comments

@mhammond
Copy link
Member

This is the server-side component of https://bugzilla.mozilla.org/show_bug.cgi?id=1204714. When the user clicks on the "please activate your account" email, we'd like the browser to receive a message after the actual activation so it can take some action (currently that action will be to have the tab switch to Sync prefs)

I've no strong opinion on the message name (but it should probably reflect that it's not a generic "user now verified" message, but instead "user verified via the activation page", which is a subtle distinction) and the message content should include the uid (and probably the email) of the user.

@rfk
Copy link
Contributor

rfk commented Sep 18, 2015

I recall @ncalexan suggesting they might need something similar for Fennec; Nick, should we triage this into the fennec-web-login milestone?

@ncalexan
Copy link
Member

@rfk yes, Fennec (and Firefox for iOS) would appreciate this. It's strictly an improvement to our existing behaviour, however, so it's not a day one blocker.

@ncalexan
Copy link
Member

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1206086 for Fennec.

@shane-tomlinson
Copy link

@ncalexan, @mhammond - I was thinking about an almost identical concept over the weekend to help support #3024 and #3081. For the use case mentioned here, instead of the "user is confirmed via the confirmation page" signal only being sent to other tabs, we'd send it out via the WebChannel as well.

@mhammond
Copy link
Member Author

Chatting with @shane-tomlinson on IRC, I think we agree that #3024 and #3081 can use the same message we need so long as they don't grow to include the use-case of verification happening in a different browser (in which case a different message would need to be used)

@ncalexan
Copy link
Member

On Mon, Sep 21, 2015 at 2:13 AM, Mark Hammond notifications@github.com
wrote:

Chatting with @shane-tomlinson https://github.com/shane-tomlinson on
IRC, I think we agree that #3024
#3024 and #3081
#3081 can use the
same message we need so long as they don't grow to include the use-case of
verification happening in a different browser (in which case a different
message would need to be used)

Can I get a summary of that chat?

I find this distinction surprising. The event I want to observe is "the
account transitioned from unverified to verified"; when phrased like that,
it's clearly not important how that came to be. You could imagine in
future we'll observe that message from Push Notifications; right now, we
might have a WC open to deliver it. When we do, I want to give the user an
upgraded experience.

@mhammond
Copy link
Member Author

Can I get a summary of that chat?
I find this distinction surprising.

Shane and I were successfully confusing each other, so a copy-paste of the conversation will not help. I think it boils down to:

  • For my use-case, I want the message only if the user transitioned to verified via a specific tab in this browser.
  • A more general use-case is a message when the user transitions to verified, regardless of how or where that happened. It sounds like this is what you are asking for ("it's clearly not important how that came to be.")

For my use-case, we want to take special action on the tab that caused the verification, but take no action on any other tabs that may happen to be on accounts.firefox.com. If the verification happened on a different browser or a different device, no special action should be taken anywhere.

For the more general use-case, imagine the "original browser" has a page on accounts.firefox.com but the user verifies on a different device/browser - the salient question is - does that "original browser" get a message about the verification? IIUC, the implementation being considered is that the verify page itself is what sends the message - so the "original browser" will not get the message as that browser never landed on the verify page. However, that smells like an implementation detail and that an obvious future enhancement might be that the "original browser" does get the message - that would then break the flow we are after.

IOW - I'm looking for a message that I know with certainty came from the verification page. That sounds subtly different than a message that indicates the user became verified, even if in the short term the implementation means they are the same in practice.

I hope that makes sense :)

@shane-tomlinson
Copy link

ref #3140 because similar decision logic will be used to decide whether to send this message and whether the sync preferences button can be shown on the verification page.

@rfk rfk added this to the FxA-0: quality milestone Oct 19, 2015
@rfk
Copy link
Contributor

rfk commented Oct 30, 2015

There's client-side UX cleanup that's blocked on this work, so I've put it at the top of our "quality" queue for the next train.

@shane-tomlinson
Copy link

client-side UX cleanup that's blocked on this work

client-side meaning browser, or client-side meaning content-server?

@shane-tomlinson
Copy link

@mhammond and @ncalexan - In #3140, we send a WebChannel message to Fennec after the user clicks a "Sync preferences" button and not automatically. This gives the user an opportunity to see the "You have completed verification" screen and make the choice whether they want to see the Sync Preferences pane.

I have implemented similar logic for desktop, where the user clicks the Sync Preferences button and desktop opens the correct pane. I actually have the desktop patch (sans tests) working.

Sync Preferences button->about:preferences#sync

To me it makes sense to use similar modes of interaction for both Fennec and Desktop, meaning I think we should show the button in both environments and give the user the choice.

@mhammond - I can whip the desktop hack I did into shape and open a patch against https://bugzilla.mozilla.org/show_bug.cgi?id=1204714.

@shane-tomlinson
Copy link

@ryanfeeley - could you give your thoughts on the last comment I made, to see if that seems reasonable to you?

@shane-tomlinson
Copy link

@mhammond and @ncalexan - If going with the above scheme where the browser only opens about:preferences#sync (or equivalent) once the user clicks the Sync preferences button, is a WebChannel message indicating the user has verified still needed, esp w/ the coming push notifications?

@shane-tomlinson
Copy link

@mhammond - FWIU, context=fx_desktop_v2 is soon going to be a thing, support for the Sync preferences button could be a part of that.

shane-tomlinson pushed a commit that referenced this issue Nov 5, 2015
…ktop_v2

The user will see the `Sync preferences` button, and if it is clicked,
a `fxaccounts:sync_preferences` WebChannel message will be sent to
the browser.

issue #3079
@rfk
Copy link
Contributor

rfk commented Nov 10, 2015

Sounds good. Is the concrete thing asked for in this bug (an outgoing message after account activation) the right way to achieve all those flows from our POV?

@shane-tomlinson
Copy link

Sounds good. Is the concrete thing asked for in this bug (an outgoing message after account activation) the right way to achieve all those flows from our POV?

I think so. It can be a "fire and forget" message. @mhammond - do you want this message sent only if the user signed up using a WebChannel flow, or should the message be sent for the old fx_desktop_v1 broker and the CustomEvent it uses?

@mhammond
Copy link
Member Author

do you want this message sent only if the user signed up using a WebChannel flow, or should the message be sent for the old fx_desktop_v1 broker and the CustomEvent it uses?

I think it should only be sent via the webchannel because:

If the user verifies in the same browser with about:accounts open,

In this scenario we'd get 2 notifications and we'd need some way of ensuring we only take the action once - which we'd probably do by ignoring the CustomEvent completely (as the webchannel message should also give us the browser element the message came from so we can change its URL)

If the user verifies in the same browser after closing about:accounts,

In this scenario we wouldn't get the CustomEvent as about:accounts is closed.

I guess this means self-hosters et al would not get this fancy behaviour until they upgraded the server, but I think that's OK - this is just minor UX and not key functionality.

@vladikoff
Copy link
Contributor

@rfk and @vladikoff to talk about this

@vladikoff
Copy link
Contributor

Hey @mhammond , let's chat about this in Orlando :D

@shane-tomlinson
Copy link

@vladikoff, @mhammond - can you leave your Orlando notes here for posterity?

@vladikoff
Copy link
Contributor

@shane-tomlinson, basically @ryanfeeley said: bring the blue Android "Sync Prefs" button from Fennec to desktop Ready page.

It would send the SYNC_PREFERENCES: 'fxaccounts:sync_preferences' to Desktop. this involves a desktop patch.

SYNC_PREFERENCES: 'fxaccounts:sync_preferences'

cc @mhammond

@vladikoff vladikoff changed the title Send webchannel message from "activate your account" page Add a "Sync Preferences" button to the account ready page [desktop sync][opened via a new tab] Dec 14, 2015
@shane-tomlinson
Copy link

@mhammond, @vladikoff - has the desktop work been completed for this? If not, I don't mind doing it.

Would this require fx_desktop_v3 or just pile onto fx_desktop_v2?

@vladikoff
Copy link
Contributor

@mhammond, @vladikoff - has the desktop work been completed for this? If not, I don't mind doing it.
Would this require fx_desktop_v3 or just pile onto fx_desktop_v2?

No desktop patch yet. we might be able to pile onto v2 if v2 is only used by nightly.

@shane-tomlinson
Copy link

No desktop patch yet. we might be able to pile onto v2 if v2 is only used by nightly.

v2 should already be in Aurora, so we'll have to uplift the functionality to 45 or use an alternative mechanism. We could 1) add a query parameter indicating "opt-in", 2) add a new context, or 3) finally do the capabilities handshake.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants