Suppress popup notifications explicitly when the window is minimized

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Site Identity and Permission Panels
P1
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox53 verified, firefox54 verified, firefox55 verified)

Details

(Whiteboard: [fxprivacy])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
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)

Updated

5 months ago
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Iteration: --- → 55.1 - Mar 20
Flags: qe-verify+
Priority: -- → P1
(Assignee)

Updated

5 months ago
Whiteboard: [fxprivacy][triage]
(Assignee)

Comment 1

5 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2712ca6a540962736fba1c653e2c75ca30415963
Comment hidden (mozreview-request)

Comment 3

5 months ago
mozreview-review
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.
Flags: needinfo?(philipp)
Duplicate of this bug: 1347063
Setting status flags as a reminder for uplift requests.
status-firefox53: --- → affected
status-firefox54: --- → affected
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)

Comment 8

4 months ago
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6690e9b920da
Suppress popup notifications explicitly when the window is minimized. r=johannh
(Assignee)

Comment 9

4 months ago
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.
Flags: needinfo?(paolo.mozmail)
(Assignee)

Updated

4 months ago
Flags: needinfo?(philipp)

Comment 10

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6690e9b920da
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Thanks for the clarification! Sounds good to me!
Flags: needinfo?(philipp)
Sorry, that was unclear: the behavior *with* the patch works :)
(Assignee)

Comment 13

4 months ago
str
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
Attachment #8844973 - Flags: approval-mozilla-beta?
Attachment #8844973 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)

Comment 15

4 months ago
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.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
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+.
Attachment #8844973 - Flags: approval-mozilla-beta?
Attachment #8844973 - Flags: approval-mozilla-beta+
Attachment #8844973 - Flags: approval-mozilla-aurora?
Attachment #8844973 - Flags: approval-mozilla-aurora+

Comment 17

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/25890bbebba0
status-firefox54: affected → fixed

Comment 18

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/cf353fe0e79e
status-firefox53: affected → fixed

Updated

4 months ago
Whiteboard: [fxprivacy][triage] → [fxprivacy]

Comment 19

4 months ago
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.
status-firefox54: fixed → verified
I also verified that the notifications hides just as Firefox is minimized using latest Firefox 53 beta 5 on Ubuntu 16.04 64bit.
status-firefox53: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.