966098 - figure out post-sign-in notification behavior for minimized/non-existent/background windows

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P2)

Description

[:Gavin Sharp [email: [e-mail]]]

Currently, we have notifications that show up as a result of you verifying your new Firefox account's email address. They're either:

a) "Sync has started!" panel anchored on the menu button
b) a modal dialog to choose your sync options (if you chose to customize what to sync)

Since verification can occur outside of Firefox, it's possible there are no Firefox windows (or no foreground windows, or no maximized windows) when these notifications occur. We should double check that in these edge cases something reasonable happens, for both a) and b).

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to :Gavin Sharp (email [e-mail]) from comment #0)
> Currently, we have notifications that show up as a result of you verifying
> your new Firefox account's email address

(or signing in)

> Since verification can occur outside of Firefox, it's possible there are no
> Firefox windows (or no foreground windows, or no maximized windows) when
> these notifications occur. We should double check that in these edge cases
> something reasonable happens, for both a) and b).

I just managed to get the "you're now syncing!" notification after signing in and switching to another app quickly. The panel appeared floating in front of my other window all on its own. That shouldn't happen!

Comment 2

[Ryan Feeley [:rfeeley]]

After a quick chat with Gavin and Madhava, UX's position is that any doorhangers and/or modal sheets be delayed until the first Firefox window is opened/maximized. In future, we may design something more sophisticated using the desktop notification engine, but this will do for now.

Comment 3

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

I'm not sure what is actionable here.  I believe that if Fx isn't running, the right thing will happen next time it does run.

In terms of Fx running but having no visible windows, I'm not sure what state can get us here.  IIUC, OSX might be able to hit this (if you just close the only open window, Fx stays running on OSX, right?) - are there any other known cases that could lead us to that state?

Comment 4

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

(In reply to Mark Hammond [:markh] from comment #3)

Oops - I missed the "minimized" part of this - but leaving needinfo as I'd still like to understand what "non existent" means, and to clarify if we really care about "background" windows - assuming that simply means when Fx is not the foreground application - where "now synching" would probably be wrong - it would have already happily synced.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

The action here is to handle minimized, "background" (i.e. other app is focused), and "no windows" (on Mac) reasonably.

- for the minimized case: show notification when unminimized, or on next startup.
- for the "background" case: show notification when window gets focus, or on next startup
- for no windows: show the notification on next window open, or on next startup

PopupNotifications.jsm has some relevant logic:
http://hg.mozilla.org/mozilla-central/annotate/04bdb7369ba4/toolkit/modules/PopupNotifications.jsm#l301

Perhaps a good idea to also call window.getAttention() in the background/minimized cases.

Comment 7

[Marco Mucci [:MarcoM]]

Being tracked in Sync - removed from Desktop Backlog.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-966098-Handle-inactive-no-windows-case-for-sync-.patch

Fixing this for the "sync started" doorhanger should be quite easy. We already don't show the doorhanger if we're not the active window. By listening for the "activate" event we can check if we still need to show it or some other window cleared the pref in the meantime. At the next startup, when receiving the sync:start notification, we will check again and show the doorhanger if we still didn't get around to do that.

Comment 9

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Comment on attachment 8404023 [details] [diff] [review]
0001-Bug-966098-Handle-inactive-no-windows-case-for-sync-.patch

Review of attachment 8404023 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't work for me on Windows, presumably as the panel gets auto-hidden as the window is restored.  If I change onSyncStart to do:

      window.setTimeout(function() {
        gFxAccounts.showSyncStartedDoorhanger();
      }, 0);

instead of calling showSyncStartedDoorhanger() directly it seems to work fine - although it does have a slightly strange visual impact as the panel is displayed in the "correct" position even though the window is still animating - ie, the panel appears *before* the window has finished restoring.  1 second or so might be even better?

comment 5 also says we should maybe call window.getAttention() - personally I'm not too bothered by that, but it might be worth pinging Gavin and see how much he cares :)

Finally, my testing shows we don't get the activate event at next startup - presumably we are adding the event handler after it has been sent.  Thus, the comment 5 requirements for "or on next startup" don't seem to be satisfied here, so I think we will want a call to onSyncStart() at initialize time.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Mark Hammond [:markh] from comment #9)
> Finally, my testing shows we don't get the activate event at next startup -
> presumably we are adding the event handler after it has been sent.  Thus,
> the comment 5 requirements for "or on next startup" don't seem to be
> satisfied here, so I think we will want a call to onSyncStart() at
> initialize time.

True, we don't receive "activate" on startup but we receive "weave:service:sync:start" as soon as sync starts which will then show the doorhanger.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Mark Hammond [:markh] from comment #9)
> This doesn't work for me on Windows, presumably as the panel gets
> auto-hidden as the window is restored.  If I change onSyncStart to do:
> 
>       window.setTimeout(function() {
>         gFxAccounts.showSyncStartedDoorhanger();
>       }, 0);
> 
> instead of calling showSyncStartedDoorhanger() directly it seems to work
> fine - although it does have a slightly strange visual impact as the panel
> is displayed in the "correct" position even though the window is still
> animating - ie, the panel appears *before* the window has finished
> restoring.  1 second or so might be even better?

Ah, nice find, thanks! I wonder if we have some way to know whether we animation has finished...

> comment 5 also says we should maybe call window.getAttention() - personally
> I'm not too bothered by that, but it might be worth pinging Gavin and see
> how much he cares :)

Hmm. Gavin, do you care? Do we want to get attention for that? It seems like waiting for the next window activation is sufficient.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-966098-Handle-inactive-no-windows-case-for-sync-.patch, v2

Ok, this seems to work pretty well for me.

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/890b26ee9940

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Tracking for 30 and 31.

Do you want to have in 29?

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

This change is very self-contained and has a very small risk but it would be absolutely great to get this into 29 as well. I can formulate my thoughts in an approval request.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

If you don't mind, after checking with Gavin on IRC, I would prefer to skip 29 for this patch. We try to limit the number of patches to critical problems or with an important user impact.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/890b26ee9940

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominate for uplift, if this is low enough risk - we're in our final week of taking more speculative work on Beta 30.

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

Yes, looks like I forgot to request approval after we decided to not take it into 29.

Comment 20

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8407546 [details] [diff] [review]
0001-Bug-966098-Handle-inactive-no-windows-case-for-sync-.patch, v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 957436
User impact if declined: Users might miss sync notifications when Firefox isn't focused / the active application.
Testing completed (on m-c, etc.): Good on Nightly and Aurora for a while.
Risk to taking this patch (and alternatives if risky): Low risk.
String or IDL/UUID changes made by this patch: None.

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/releases/mozilla-beta/rev/937f11ea90b2

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/937f11ea90b2

Comment 23

[Andrei Vaida [:avaida]]

These are the scenarios I targeted with my tests on Firefox 30.0 build2 (Build ID: 20140605174243):
 1. Sync account verified
  1.1. Firefox is *not* running,
  1.2. Firefox window is *not* in focus,
  1.3. Firefox window is in focus,
  1.4. Firefox window is minimized,
 2. First login with a Sync account
  2.1. Firefox window is *not* in focus,
  2.2. Firefox window is in focus,
  2.3. Firefox window is minimized,
across the following platforms:
 * Windows 7 64-bit [1],
 * Ubuntu 13.10 64-bit [2],
 * Mac OS X 10.9.2 [3].

Most of these cases have the expected behavior, according to Gavin and Mark's notes, but the following ones don't:
* Windows 7 64-bit: 
** The anchored panel is *not* displayed if the Firefox Account was verified while the browser was not running and "Choose what to Sync" was *not* checked.
* Ubuntu 13.10 64-bit: 
** The anchored panel is *not* displayed if the Firefox Account was verified while the browser was not running and "Choose what to Sync" was *not* checked.
** The anchored panel is *not* displayed on first login, if the Firefox window is *not* in focus.

Please let me know what are your thoughts on this and if there are any other scenarios I should take a look at here.


[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0
[2] Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0
[3] Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0

Comment 24

[Tim Taubert [:ttaubert] (inactive)]

I'm not sure why those would work on OS X but not on Windows/Linux. I think we should file follow-ups for those two cases and investigate what's going wrong there.

Comment 25

[Andrei Vaida [:avaida]]

Following a more in-depth investigation, it seems that the issue affecting Windows 7 64-bit was a false positive, the anchored panel is in fact displayed after 15-20 seconds since browser start-up, sometimes after a bit longer.

The behavior inconsistencies from Linux on the other hand are a different story:
> ** The anchored panel is *not* displayed if the Firefox Account was verified
> while the browser was not running and "Choose what to Sync" was *not*
> checked.
Upon launch, Firefox is displayed at a specific location on the screen and at a specific window size, according to the previous session. If you don't do anything with the browser for a few seconds (and I mean nothing at all), the anchored notification will be displayed, but if you so much click the browser's title bar or move the window once, the anchored notification will *not* be displayed. 

> ** The anchored panel is *not* displayed on first login, if the Firefox
> window is *not* in focus.
This is 100% reproducible on my Ubuntu machine but I suspect it's related to what I mentioned above as well.

Tim, I'm not sure if I'm on the right lead here but I'll file new bugs for both of these two issues. If you have any thoughts on the notes above, please let me know.

Comment 26

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Andrei Vaida, QA [:avaida] from comment #25)
> Following a more in-depth investigation, it seems that the issue affecting
> Windows 7 64-bit was a false positive, the anchored panel is in fact
> displayed after 15-20 seconds since browser start-up, sometimes after a bit
> longer.

Right, Sync waits 10-15s until it initializes and thus it takes a little longer after startup for the message to appear.

> Upon launch, Firefox is displayed at a specific location on the screen and
> at a specific window size, according to the previous session. If you don't
> do anything with the browser for a few seconds (and I mean nothing at all),
> the anchored notification will be displayed, but if you so much click the
> browser's title bar or move the window once, the anchored notification will
> *not* be displayed. 

That sounds weird and worth filing a bug for.

> > ** The anchored panel is *not* displayed on first login, if the Firefox
> > window is *not* in focus.
> This is 100% reproducible on my Ubuntu machine but I suspect it's related to
> what I mentioned above as well.

Does it display the message a few seconds after the window received focus? It doesn't show the doorhanger for unfocused messages by design.

Comment 27

[Andrei Vaida [:avaida]]

(In reply to Tim Taubert [:ttaubert] from comment #26)
> Does it display the message a few seconds after the window received focus?
> It doesn't show the doorhanger for unfocused messages by design.

I'm aware of that, but the message is not displayed at all even after bringing the browser's window into focus.

Comment 28

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Andrei Vaida, QA [:avaida] from comment #27)
> I'm aware of that, but the message is not displayed at all even after
> bringing the browser's window into focus.

Ok, then let's please investigate this in a follow-up too. Thanks!