990042 - Sync Promo banner shown after account addition

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Currently we continue to show the Sync promo banner after an account addition when accessing Sync not through the banner 

* Start Firefox with a new profile
* Settings → Sync (create or add an account)
* Tap 'Back to Browsing'

See Sync promo banner still shown

After an account is added, tapping the banner does nothing but merely dismiss it. 

That particular promotion should listen for an account addition when initiated through non-banner entryway.

Comment 1

[:Margaret Leibovic]

We would need to change the Accounts.jsm API to add a listener for this.

The banner will be gone the next time the user launches the browser, and they can still close it with the x, so I don't think this is something we need to rush for 29, but it could be a good enhancement if it doesn't add too much complexity.

Comment 2

[Richard Newman [:rnewman]]

An interim solution is to re-run the "should I show this?" check for the current banner just after onResume, hiding it if the check now fails. You'll see the banner disappear, but at least it'll be gone.

Comment 3

[Richard Newman [:rnewman]]

I think Comment 2 is mentorable.

Comment 4

[Michael LoPiccolo (:milo)]

I'll look at this one!

Right now I've just been piecing together how all these parts communicate (Snippets.js, Home.jsm, HomeBanner.java). Working toward the fix in Comment 2.

Comment 5

[Michael LoPiccolo (:milo)]

Alright, I've had some time to look at this but I haven't gotten that far. Gonna need mentor help.

It looks to me like there are two issues here:

1. We need to refresh the list of snippets after a Sync account is created.
So, I need a way to trigger the code at Snippets.js:415-424 after creating a Sync account. (We could also trigger it every onResume(), as in Comment 2, but that is a little messy). I'm not familiar with Sync or with Gecko messaging, so I'll need some help there.

We also might as well "browser.snippets.syncPromo.enabled" to false when a Sync account is created - this is easy, we can just put it before Snippets.js:350, for example.

2. We need to get the HomeBanner to refresh itself (HomeBanner.java's update()? ) when a Sync account is created. Once again, we could just do this every onResume(). Right now, it updates every time we close and reload the Awesomescreen, which is already pretty good - the problem is it's picking a snippet from the old set which still includes the sync promo.



Let me know if that sounds like a good blueprint! If it does, I'll start asking about the nitty-gritty details.

I'm not very familiar with any of this, so it'll definitely take a bunch of mentor effort to get this done. I won't take it personally if you want to WONTFIX this and spend time on more significant bugs.

Comment 6

[:Margaret Leibovic]

Sorry for the slow response! I was traveling, and I'm just catching up on bugzilla requests now.

The tricky thing about trying to hook something into onResume is that we would need to send a message from Java to JS to do that, and that feels a bit heavy-handed, especially since Snippets.js doesn't currently have any messaging code in it. Richard, how exactly did you imagine this working?

The home banner has a `remove` API, so it would be nice to try to explicitly remove the sync promo banner when we refresh the account state, but that would mean we also need to hang onto the id that's created in the Home.banner.add call.

I'm tempted to just WONTFIX this bug, unless Richard can propose a compelling solution.

I'm sorry you spent time looking into this! I'd love to help you find another bug to work on!

Comment 7

[Michael LoPiccolo (:milo)]

It's cool! I'm a little relieved to hear that I didn't miss a super obvious solution, actually.

I agree that a WONTFIX is probably the right decision - we could fix this, but it would add a ton of code complexity for a relatively small fix. Unless there will be many more Snippets with this kind of functionality, it's probably not worth it.
Pending Richard's input, of course! Maybe there's an easy partial fix we overlooked.

Comment 8

[Richard Newman [:rnewman]]

(In reply to :Margaret Leibovic from comment #6)

> The tricky thing about trying to hook something into onResume is that we
> would need to send a message from Java to JS to do that, and that feels a
> bit heavy-handed, especially since Snippets.js doesn't currently have any
> messaging code in it. Richard, how exactly did you imagine this working?

One of a few ways.

* Have FxA routinely message JS when an account is created; I can't imagine that we won't use that for other things. If we know in JS-land that we just displayed the Sync banner, and we get that message, we can show another or dismiss this one.

and two that don't involve messaging:

* Always dismiss the Sync snippet after tapping it. This can be pure Java -- just get rid of the UI element altogether for the duration of the session!

* Presumably the JS side knows when we just loaded about:home -- about:home is an XHTML page behind the scenes. Re-check a second after load completes.

Comment 9

[Michael LoPiccolo (:milo)]

(In reply to Richard Newman [:rnewman] from comment #8)
> 
> * Always dismiss the Sync snippet after tapping it. This can be pure Java --
> just get rid of the UI element altogether for the duration of the session!
> 

I'm not sure I understand this - the snippet is always dismissed when it's tapped. The problem is when the user creates a Sync account through the settings menus, not the promo snippet.

The ideas sound promising, but I'm afraid I'll have to unassign myself for now - I won't be able to put much time into Mozilla things for some time. Thanks for your help!

Comment 10

[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.