After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 678121 - "Your chat status will be set to busy" notification not shown when there's another
"Your chat status will be set to busy" notification not shown when there's an...
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: message-tray
3.5.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-14 18:46 UTC by Cosimo Cecchi
Modified: 2015-02-23 22:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix bug 678121 (70.74 KB, patch)
2012-06-21 12:48 UTC, Carlos Soriano
none Details | Review
Fix bug 678121 (70.06 KB, patch)
2012-06-21 12:55 UTC, Carlos Soriano
needs-work Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. We need this modification because until now, if some notif (3.97 KB, patch)
2012-06-21 22:31 UTC, Carlos Soriano
none Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. (2.05 KB, patch)
2012-06-21 22:55 UTC, Carlos Soriano
none Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. (2.05 KB, patch)
2012-06-21 22:59 UTC, Carlos Soriano
none Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. (4.31 KB, patch)
2012-06-21 23:13 UTC, Carlos Soriano
none Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. (4.02 KB, patch)
2012-06-21 23:27 UTC, Carlos Soriano
needs-work Details | Review
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. (4.02 KB, patch)
2012-06-21 23:36 UTC, Carlos Soriano
none Details | Review

Description Cosimo Cecchi 2012-06-14 18:46:19 UTC
- trigger a notification
- while the notification is still showing, flip the "Notifications" switch off in the user menu
- flip back the notification on
- the "Your chat status will be set to busy" banner will be shown now after the switch has been turned back on

We should either avoid showing that banner if there's another notification displayed, or just always show it and override any existing notifications (I tend towards the latter). Showing it after the status is not busy anymore is just a lie :)
Comment 1 Carlos Soriano 2012-06-21 12:48:09 UTC
Created attachment 216913 [details] [review]
Fix bug 678121
Comment 2 Carlos Soriano 2012-06-21 12:55:37 UTC
Created attachment 216914 [details] [review]
Fix bug 678121
Comment 3 Carlos Soriano 2012-06-21 12:57:38 UTC
ups, I'm doing very bad diff, give me some time, it's my first patch.
Comment 4 Florian Müllner 2012-06-21 17:00:01 UTC
Review of attachment 216914 [details] [review]:

No idea what this is, but almost the entire patch are whitespace fixes for issues that are not present upstream. If you attach a proper patch, please give it a proper commit message - http://lists.cairographics.org/archives/cairo/2008-September/015092.html provides good guidelines.
Comment 5 Carlos Soriano 2012-06-21 20:42:04 UTC
Yes, I format the entire document accidentally, and I did a commit. After that I go to Head revision and modify the code that is necessary to do a patch and did another commit, but the diff show all the changes. That's why I'm doing it incorrectly. But now I learnt how to merge all commits into one, and after this I will push a properly patch.
As I said, please give some time, I will take notice you when a patch is correctly for me.

Thank you for patience.
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-06-21 20:49:11 UTC
Note that almost all search providers will be external in the 3.6 cycle, which should eliminate a large amount of issues.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-06-21 21:00:59 UTC
Er, whoops, I put this on the wrong bug. This was meant to go on bug #678571
Comment 8 Carlos Soriano 2012-06-21 22:31:41 UTC
Created attachment 216973 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled. We need this modification because until now, if some notif
Comment 9 Carlos Soriano 2012-06-21 22:55:01 UTC
Created attachment 216974 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled.

We need this modification because until now, if some notification is
triggered and we deactivate notifications, and activate the
notifications again, the notification warning the user that
notifications will be disabled is shown, and after enabling
notifications this message is a lie.

Bug related: https://bugzilla.gnome.org/show_bug.cgi?id=678121
Comment 10 Carlos Soriano 2012-06-21 22:59:53 UTC
Created attachment 216975 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled.

We need this modification because until now, if some notification is
triggered and we deactivate notifications, and activate the
notifications again, the notification warning the user that
notifications will be disabled is shown, and after enabling
notifications this message is a lie.

Bug related:
Comment 11 Carlos Soriano 2012-06-21 23:13:24 UTC
Created attachment 216976 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled.

We need this modification because until now, if some notification is
triggered and we deactivate notifications, and activate the
notifications again, the notification warning the user that
notifications will be disabled is shown, and after enabling
notifications this message is a lie.

Bug related:
Comment 12 Carlos Soriano 2012-06-21 23:27:50 UTC
Created attachment 216977 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled.

We need this modification because until now, if some notification is
triggered and we deactivate notifications, and activate the
notifications again, the notification warning the user that
notifications will be disabled is shown, and after enabling
notifications this message is a lie.

Bug related: https://bugzilla.gnome.org/show_bug.cgi?id=678121
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-06-21 23:33:00 UTC
Review of attachment 216977 [details] [review]:

Did you test this? It causes a syntax error for me.
Comment 14 Carlos Soriano 2012-06-21 23:36:48 UTC
Created attachment 216979 [details] [review]
Add function to warn the user that notifications will be disabled from now on. Also overrides current notification showing(if it exists) to display immediately that notifications will be disabled.

We need this modification because until now, if some notification is
triggered and we deactivate notifications, and activate the
notifications again, the notification warning the user that
notifications will be disabled is shown, and after enabling
notifications this message is a lie.

Bug related: https://bugzilla.gnome.org/show_bug.cgi?id=678121
Comment 15 Carlos Soriano 2012-06-21 23:40:06 UTC
Jasper, sorry for that, cause my unknowledge I had errors with bz tool and yes, bz put in my code some code like <<<<<HEAD<<<< or something like this, and this cause sintax errors, and I was doing a lot of changes with bz, git, etc...

Now I think that I learnt how to work with it in a good way.

Now you can test, my last patch is for me good.

Thank you for the patience and sorry for the spam.
Comment 16 Allan Day 2012-11-22 11:09:20 UTC
Maybe I'm being stupid, but why do we need to special case this notification? Our notifications framework should really be able to handle this, either by (1) queuing the banner as normal or (2) treating the notification as urgent.
Comment 17 Jasper St. Pierre (not reading bugmail) 2012-11-22 11:12:19 UTC
(In reply to comment #0)
> - trigger a notification
> - while the notification is still showing, flip the "Notifications" switch off
> in the user menu
> - flip back the notification on
> - the "Your chat status will be set to busy" banner will be shown now after the
> switch has been turned back on
> 
> We should either avoid showing that banner if there's another notification
> displayed, or just always show it and override any existing notifications (I
> tend towards the latter). Showing it after the status is not busy anymore is
> just a lie :)

What about 3) Destroy the banner when you flip the switch back on?
Comment 18 Carlos Soriano 2012-11-22 11:54:19 UTC
Allan,

Because this notification has to be the first, before urgent notifications, because is a immediate feedback. It is not?
Imagine you have a cue with 10 urgent notifications, you switch off the notifications, then i.e. 3 urgent notifications is shown in this period. Then you switch on the notifications again, and after 7 notifications more the "Your chat status will be set to busy" text is shown. It is not correct.

Jasper,

Imagine the situation when a transient notification is triggered automatically and 0.0001 seconds after you switch to off the notifications button and you delete the banner, you lost the notification, it is not?

All of these are edge cases, but anyway, we have to manage all cases. In the last patch I attached I manage this correctly.
Comment 19 Jasper St. Pierre (not reading bugmail) 2012-11-22 17:25:08 UTC
All I'm saying is that if you toggle notifications off, it queues up a notification saying you're busy, and when you toggle notifications on, it deletes that notification. Doesn't matter about transients or anything else.
Comment 20 Carlos Soriano 2012-11-22 18:37:27 UTC
Jasper,
Ah ok, you're saying to delete the "Your chat status will be set to busy" notification if you switch notifications to on before this notification is shown.

I understand. It's a good solution. But IMHO I prefer to show directly, because I think it requires a immediate feedback, but well, you choose :)
Comment 21 Florian Müllner 2012-11-22 18:50:32 UTC
For what it's worth, there have been some discussions lately to drop the presence <-> notifications interaction entirely. If we go that route, we will be able to drop the notification altogether ...
Comment 22 Jasper St. Pierre (not reading bugmail) 2012-11-22 18:57:01 UTC
Yeah, we originally had a "Do Not Disturb" switch, which was swapped out for a "Notifications" switch. Which makes a lot more sense.
Comment 23 Florian Müllner 2015-02-23 22:39:23 UTC
That notification has been gone since early 2013 ...