1184790 - Notification rectangle is completely gray if mouse was placed over another application when notification appeared AND mouse was placed in the notification area

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect)

Description

[arni2033]

Attachment: Notification rectangle is completely gray - testpage.html

(Tested on Windows 7 with Fx39,41,42 )

STR:
1. Open testpage; change the firefox window size so that the area (rectangle) where notification appears, was not placed over any current firefox windows
2. Click "Start countdown" and move mouse in the notification area before the countdown is finished

Result:
Gray reactangle appeared instead of notification (actually if you move mouse away from notification area, then animation will continue and you'll actually see the notification)

Comment 1

[arni2033]

Attachment: Notification rectangle is completely gray - screencast.webm

Comment 2

[arni2033]

Expectations:
Normal notification should've appeared, regardless of mouse position

Comment 3

[Alice0775 White]

I can reproduce the problem

Comment 4

[Alice0775 White]

And this is a regression since Firefox27.

Comment 5

[Alice0775 White]

Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=184a0076278b&tochange=220ce292a894

Suspect:
	51d151978c2b	Matthew Noorenberghe — Bug 888380 - Alert service notifications shouldn't disappear when hovered. r=jaws

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

I'm pretty sure that there is already a bug on file for this. But to fix this we need to let the notification fade in before stopping the animation.

Comment 7

[arni2033]

Also, I think it's a good idea to show the actual notification, not the gray rectangle, if animation is stopped in its end (user hovered mouse over notification just before it disappeared). In this case, notification is also just a gray rectangle.
Please tell me if this deserves another bug report OR create it yourself if there's no dupe for this

Comment 8

[Matthew N. [:MattN]]

I think this is fixed now on Fx44+, do you agree?

Comment 9

[arni2033]

(In reply to Matthew N. [:MattN] from comment #8)
> I think this is fixed now on Fx44+, do you agree?
This is not fixed at all (including comment 7). Now I don't see gray rectangle, but
the original bug was that notification was unreadable with almost zero opacity. In more detail:
1) opacity 0->1 transition didn't finish
   when I hovered mouse over notification in the start of the transition
2) opacity 1->0 transition didn't came back to 1
   when I hovered mouse over notification in the end of the transition (comment 7)

I can provide screencast of each behavior via my dropbox.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Yeah I don't suspect that this is fixed. The grey box should no longer be there as the notifications are now transparent, but this just means that the notification will not fade-in until the mouse is moved. We likely need to adjust the CSS rule that pauses the animation so it doesn't apply until the notification has made it to the second step of the CSS Animation (though I don't think the CSS Animation spec has anything like this).

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1184790 - Force notifications to have opacity:1 when hovered to make sure that we show a readable notification during the intro/exit animation of the notification if the user has their mouse in the area.

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

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 8791649 [details]
Bug 1184790 - Force notifications to have opacity:1 when hovered to make sure that we show a readable notification during the intro/exit animation of the notification if the user has their mouse in the area.

https://reviewboard.mozilla.org/r/79020/#review77612

::: toolkit/themes/shared/alert-common.css:29
(Diff revision 1)
>  #alertBox[animate][closing] {
>    animation-duration: .6s;
>    animation-name: alert-closing-animation;
>  }
>  
> +#alertBox[animate]:not([clicked]):not([closing]):hover {

OK, I see this is the same selector as the one used for `animation-play-state: paused;`. LGTM

::: toolkit/themes/shared/alert-common.css:30
(Diff revision 1)
>    animation-duration: .6s;
>    animation-name: alert-closing-animation;
>  }
>  
> +#alertBox[animate]:not([clicked]):not([closing]):hover {
> +  /* !important is necesary because CSS animations have highest

Spelling nit: necessary

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8791649 [details]
Bug 1184790 - Force notifications to have opacity:1 when hovered to make sure that we show a readable notification during the intro/exit animation of the notification if the user has their mouse in the area.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79020/diff/1-2/

Comment 14

[arni2033]

@ Jared Wein:
Could you please check if this patch causes the following bugs? (+ are they real bugs)

1) When push notification appears below moving mouse pointer,it appears w/o opacity fade-in transition
2) When I hover mouse over notification in the end of transition, then move mouse away,
   notification disappears w/o opacity fade-out transition.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ac7de633609
Force notifications to have opacity:1 when hovered to make sure that we show a readable notification during the intro/exit animation of the notification if the user has their mouse in the area. r=MattN

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to arni2033 from comment #14)
> @ Jared Wein:
> Could you please check if this patch causes the following bugs? (+ are they
> real bugs)
> 
> 1) When push notification appears below moving mouse pointer,it appears w/o
> opacity fade-in transition
> 2) When I hover mouse over notification in the end of transition, then move
> mouse away,
>    notification disappears w/o opacity fade-out transition.

Yes the patch causes both of those behaviors. Whether they are a bug or not is up to opinion.

How would you like this to behave? I am not sure if we can transition do a transition here and I'm also not sure if we can delay or restart the notification timeout.

Comment 17

[arni2033]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Yes the patch causes both of those behaviors. Whether they are a bug or not is up to opinion.
"Sometimes push notification appears smoothly, sometimes it just shows up immediately
 I'm a simple user, and can't understand, why it's doing that, but I'd like it to be less random"

> How would you like this to behave? I am not sure if we can transition do a transition here and
> I'm also not sure if we can delay or restart the notification timeout.
It's hard to describe with words (not mock-ups), but I have no time for mock-ups now.
I have 2 alternative expectations

A) Behavior currently maintained by Mozilla, but w/o blinking
- When notification appears, it should go from opacity:0 to opacity:1, and nothing should interfere
- Special variable S invisible in UI should be set to 0
- Every millisecond when mouse is placed outside of notification, S should be increased by 1
- When S becomes greater than predefined timeout T, notification switches to "fade out" mode.
-- In "fade out" mode notification should smoothly go from current opacity to opacity:0
-- If mouse is hovered above notification, it should go from current opacity to opacity:1,
-- with the same speed (backwards animation function), and stop at opacity:1
-- When notification reaches opacity:0, it should be deleted

B) Best UX ever. Notification hides after T milliseconds after the last interaction
- When notification appears, it should go from opacity:0 to opacity:1, and nothing should interfere
- Special variable S invisible in UI should be set to 0
- Every millisecond when mouse is placed outside of notification, S should be increased by 1
* When mouse is placed above notification, S should be set to 0, i.e. reset (!)
- When S becomes greater than predefined timeout T, notification switches to "fade out" mode.
-- In "fade out" mode notification should smoothly go from current opacity to opacity:0
-- If mouse is hovered above notification, it should go from current opacity to opacity:1,
-- with the same speed (backwards animation function), and stop at opacity:1
-- When notification reaches opacity:0, it should be deleted


This is how I expect such widget to work, implementation may be different.
I'm not sure what I used as reference for (B) - perhaps it was tooltips from tray in Windows Taskbar

Comment 18

[arni2033]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Yes the patch causes both of those behaviors. Whether they are a bug or not is up to opinion.
"Sometimes push notification appears smoothly, sometimes it just shows up immediately
 I'm a simple user, and can't understand, why it's doing that, but I'd like it to be less random"

> How would you like this to behave? I am not sure if we can transition do a transition here and
> I'm also not sure if we can delay or restart the notification timeout.
It's hard to describe with words (not mock-ups), but I have no time for mock-ups now.
I have 2 alternative expectations

A) Behavior currently maintained by Mozilla, but w/o blinking
- When notification appears, it should go from opacity:0 to opacity:1, and nothing should interfere
- Special variable S invisible in UI should be set to 0
- Every millisecond when mouse is placed outside of notification, S should be increased by 1
- When S becomes greater than predefined timeout T, notification switches to "fade out" mode.
-- In "fade out" mode notification should smoothly go from current opacity to opacity:0
-- If mouse is hovered above notification, it should go from current opacity to opacity:1,
   with the same speed (backwards animation function), and stop at opacity:1
-- When notification reaches opacity:0, it should be deleted

B) Best UX ever. Notification hides after T milliseconds after the last interaction
- When notification appears, it should go from opacity:0 to opacity:1, and nothing should interfere
- Special variable S invisible in UI should be set to 0
- Every millisecond when mouse is placed outside of notification, S should be increased by 1
* When mouse is placed above notification, S should be set to 0, i.e. reset (!)
- When S becomes greater than predefined timeout T, notification switches to "fade out" mode.
-- In "fade out" mode notification should smoothly go from current opacity to opacity:0
** If mouse is hovered above notification, S should be set to 0.
   Notification should go from current opacity to opacity:1, with the same speed (backwards
   animation function), and stop at opacity:1. Nothing should interfere
   When notification reaches opacity:1, it should switch to normal (not "fade out") mode.
-- When notification reaches opacity:0, it should be deleted


This is how I expect such widget to work, implementation may be different.
«*»s represent differences between (A) and (B). «--»s represent rules of "fade out" mode.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5ac7de633609

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to arni2033 from comment #18)

I agree with both of your recommendations. I have tried to implement A but don't see a way how to, and B I think might be possible but would require some extra JavaScript code. Can you file a bug for option B? It's not high enough on my priority list right now for me to work on it but maybe I'll have time in the future or someone else can get to it sooner.