Open Bug 990042 Opened 6 years ago Updated 4 years ago

Sync Promo banner shown after account addition

Categories

(Firefox for Android :: General, defect)

29 Branch
All
Android
defect
Not set

Tracking

()

Tracking Status
firefox29 --- affected
firefox30 --- affected
firefox31 --- affected
firefox32 --- affected
firefox33 --- affected
firefox34 --- affected
firefox36 --- affected
firefox38 --- affected
firefox39 --- affected
firefox40 --- affected
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected
firefox44 --- affected
firefox45 --- affected
firefox46 --- affected
firefox47 --- affected
firefox48 --- affected
fennec - ---

People

(Reporter: aaronmt, Unassigned, Mentored)

References

Details

(Whiteboard: [good next bug][lang=js])

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.
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.
Blocks: 958891
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.
Hardware: ARM → All
I think Comment 2 is mentorable.
Mentor: margaret.leibovic
Whiteboard: [good first bug][lang=js]
Whiteboard: [good first bug][lang=js] → [good next bug][lang=js]
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.
Assignee: nobody → m.lopiccolo3
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.
Flags: needinfo?(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!
Flags: needinfo?(margaret.leibovic) → needinfo?(rnewman)
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.
(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.
Flags: needinfo?(rnewman)
(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!
Assignee: m.lopiccolo3 → nobody
tracking-fennec: --- → ?
tracking-fennec: ? → -
You need to log in before you can comment on or make changes to this bug.