1345449 - Suppress popup notifications explicitly when the window is minimized

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[:Paolo Amadini]

While "noautohide" panels are minimized automatically when the parent window is minimized, popup notifications don't use "noautohide" properly on Linux because of bug 545265. This will be fixed in bug 1320361 once bug 545265 is resolved.

In the meantime, we need to hide popup notifications manually when the window is minimized. This has the side effect of causing the panel opening animation to be displayed again when the window is restored, after the window opening animation.

Comment 1

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2712ca6a540962736fba1c653e2c75ca30415963

Comment 2

[:Paolo Amadini]

Attachment: Bug 1345449 - Suppress popup notifications explicitly when the window is minimized.

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

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8844973 [details]
Bug 1345449 - Suppress popup notifications explicitly when the window is minimized.

https://reviewboard.mozilla.org/r/118228/#review120456

The code seems good so I'll give it an r+, however there's one problem I'd like to note:

One thing I noticed on Windows was that the remember password doorhanger used to hide the "show password" option on minimize and with this patch it does not anymore. I think it's supposed to hide the option when you initially close the doorhanger and that automatically happened on minimize. Now it does not anymore so the option stays. I'm really not sure how relevant this is, so it's not blocking review from my side but we should be aware and clarify the wanted behavior for this feature e.g. with MattN or pdol.

Comment 4

[Peter Dolanjski [:pdol]]

Philipp, we'll likely need your input on this one.  Paolo may need to explain it to you, so feel free to ping him.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Setting status flags as a reminder for uplift requests.

Comment 7

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

I'm not quite sure I understand what the issue here is. Nightly seems to behave fine (but maybe I'm looking at the wrong thing...)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6690e9b920da
Suppress popup notifications explicitly when the window is minimized. r=johannh

Comment 9

[:Paolo Amadini]

You need the tryserver build from comment 1 to test this.

The "remember password" doorhanger can be closed by clicking anywhere else in the window or by navigating to another page in the website. When this happens, the doorhanger is still available, just hidden, and it can be reopened using the small icon in the address bar.

When the doorhanger first appears, we display a "show password" checkbox. When the doorhanger is reopened from the icon, we don't give the option to show the password anymore, and I believe the use case is that we don't want the password to continue to be accessible to other users of the computer, in case the user who logged in walked away and is unaware that the icon in the location bar can reopen the panel.

Now, the interesting case for discussion is when the "remember password" doorhanger is visible, and the window is minimized and then restored. Without the patch, after restoring the window the doorhanger is not visible anymore. With the patch, after restoring the window the doorhanger is visible, and it still has the "show password" checkbox.

If we need to change this and hide the checkbox, we should file a different bug, otherwise there is no action needed.

I've landed the patch in this bug already because it fixes a different issue on Linux, and we have enough duplicates to make me think that the issue may be annoying in practice.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6690e9b920da

Comment 11

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Thanks for the clarification! Sounds good to me!

Comment 12

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Sorry, that was unclear: the behavior *with* the patch works :)

Comment 13

[:Paolo Amadini]

Comment on attachment 8844973 [details]
Bug 1345449 - Suppress popup notifications explicitly when the window is minimized.

Approval Request Comment
[Feature/Bug causing the regression]: Permission Notifications
[User impact if declined]: On Linux, notifications will remain visible on the screen when the window is minimized to switch to another application. The notification can only be hidden by making a choice. We had a few duplicate reports of this bug.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Waiting for verification
[Needs manual test from QE? If yes, steps to reproduce]: On Linux, display a persistent notification such as those available from <https://permission.site>, then minimize the window and check that the noitification is also hidden.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Slightly risky for Beta
[Why is the change risky/not risky?]: We don't have specific regression tests for this case, however the is very easy to reproduce manually.
[String changes made/needed]: None

Comment 14

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 15

[Hani Yacoub, Desktop QA]

Build ID: 20170319110229
User Agent: Mozilla/5.0(X11;Linux x86_64;rv55.0)Gecko/20100101 Firefox/55.0

Verified as fixed on Firefox Nightly 55.0a1 on Ubuntu 16.04 x64.

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8844973 [details]
Bug 1345449 - Suppress popup notifications explicitly when the window is minimized.

Fix an UI notification issue when the window is minimized and was verified. Aurora54+ & Beta53+.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/25890bbebba0

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cf353fe0e79e

Comment 19

[Hani Yacoub, Desktop QA]

Build ID: 20170321004003
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

Verified as fixed on Firefox Aurora 54.0a2 on Ubuntu 16.04 x64.

Comment 20

[Bogdan Maris, Desktop QA]

I also verified that the notifications hides just as Firefox is minimized using latest Firefox 53 beta 5 on Ubuntu 16.04 64bit.