Closed Bug 1345449 Opened 3 years ago Closed 3 years ago
Suppress popup notifications explicitly when the window is minimized
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.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Iteration: --- → 55.1 - Mar 20
Priority: -- → P1
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.
Attachment #8844973 - Flags: review?(jhofmann) → review+
Philipp, we'll likely need your input on this one. Paolo may need to explain it to you, so feel free to ping him.
Setting status flags as a reminder for uplift requests.
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...)
Flags: needinfo?(philipp) → needinfo?(paolo.mozmail)
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6690e9b920da Suppress popup notifications explicitly when the window is minimized. r=johannh
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.
Thanks for the clarification! Sounds good to me!
Sorry, that was unclear: the behavior *with* the patch works :)
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
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
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 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+.
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.
I also verified that the notifications hides just as Firefox is minimized using latest Firefox 53 beta 5 on Ubuntu 16.04 64bit.
You need to log in before you can comment on or make changes to this bug.