[Patch] Remove the icon attachment when showing the battery capacity warning

View: New views
10 Messages — Rating Filter:   Alert me  

[Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hello there,

When I used to use GPM version 2.24 in Fedora 9, I always found that the
notification indication a low battery capacity was in the right place
where I expected it to be(at least the location was constant). However
once I switched to Fedora 11, all of a sudden the notification was
showing up somewhere just floating, pointing to no apparent icon, I
suppose this is because the notification is shown before the gnome power
manager icon can be shown on the notification area.

Therefore I decided to create a patch where the status icon pointer is
temporarily set to NULL while showing the capacity warning, but is
restored once the notification is shown which at least makes the
notification come up somewhere sensible.

By the way, I hope this is the right place to propose patches, if I made
a mistake, my apologies for having done so.:)

Regards,
Pramod Dematagoda

[show_capacity_warning_without_icon_attachments.patch]

Index: gpm-notify.c
===================================================================
--- gpm-notify.c (revision 3399)
+++ gpm-notify.c (working copy)
@@ -317,6 +317,7 @@
 {
  gchar *msg;
  const gchar *title;
+ GtkStatusIcon *backup_ptr;
 
  /* don't show when running under GDM */
  if (g_getenv ("RUNNING_UNDER_GDM") != NULL) {
@@ -329,6 +330,15 @@
  "which means that it may be old or broken."),
        capacity);
 
+ /*
+ * Set the status icon pointer as NULL so that the notification
+ * is shown where it is expected and not just floating about some place.
+ *
+ * After the notification is shown, the status icon is restored.
+ */
+ backup_ptr = notify->priv->status_icon;
+ notify->priv->status_icon = NULL;
+
  gpm_notify_create (notify, title, msg, GPM_NOTIFY_TIMEOUT_LONG,
    GTK_STOCK_DIALOG_WARNING,
    GPM_NOTIFY_URGENCY_CRITICAL);
@@ -342,6 +352,9 @@
                                  notify, NULL);
 
  gpm_notify_show (notify);
+
+ notify->priv->status_icon = backup_ptr;
+
  g_free (msg);
  return TRUE;
 }


_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sun, 2009-05-10 at 12:03 +0530, Pramod Dematagoda wrote:

> Hello there,
>
> When I used to use GPM version 2.24 in Fedora 9, I always found that the
> notification indication a low battery capacity was in the right place
> where I expected it to be(at least the location was constant). However
> once I switched to Fedora 11, all of a sudden the notification was
> showing up somewhere just floating, pointing to no apparent icon, I
> suppose this is because the notification is shown before the gnome power
> manager icon can be shown on the notification area.
>
> Therefore I decided to create a patch where the status icon pointer is
> temporarily set to NULL while showing the capacity warning, but is
> restored once the notification is shown which at least makes the
> notification come up somewhere sensible.
>
> By the way, I hope this is the right place to propose patches, if I made
> a mistake, my apologies for having done so.:)
>
> Regards,
> Pramod Dematagoda

Hello again, just wondering if I could get some feedback related to the
patch I sent. Is the patch bad or am I doing things the wrong way by
submitting patches here? I would just really appreciate some sort of
reply here.:)

Regards,
Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Parent Message unknown Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, 2009-05-15 at 15:56 +0100, Richard Hughes wrote:

> On Fri, May 15, 2009 at 3:47 PM, Pramod Dematagoda
> <pmd.lotr.gandalf@...> wrote:
> > Hello again, just wondering if I could get some feedback related to the
> > patch I sent. Is the patch bad or am I doing things the wrong way by
> > submitting patches here? I would just really appreciate some sort of
> > reply here.:)
>
> Sorry for the late reply -- your original message fell through the cracks.
>
> Your patch isn't correct to play with the pointers like that -- a much
> better fix would be to work out why the notification is pointing at
> the correct thing, and to fix notification-daemon. It's probably best
> to test Fedora 11 first, to see if the bug has already been fixed.
> Thanks.
>
> Richard.

Hello Richard,

I am already on Fedora 11 with all updates applied, so the problem is
still there. About the patch in general, I agree that the patch is
something I am a little unsure of as well.

But the situation is this, this notification is being shown when GNOME
starts loading, and in my laptop, this is when there is nothing visible
in the screen at all apart from the wallpaper, so the notification gets
attached to an icon which does not seem to exist at that time, which
probably explains why the notification is just seen floating instead of
showing up somewhere sensible. So this problem may not necessarily be
something wrong with notification-daemon but with the issue that GPM
shows the notification just too quickly to be attached to a present
icon. If you could suggest what would be best in this case, I'll see
what I can do. :)

Regards,
Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Richard Hughes-6 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, May 15, 2009 at 4:34 PM, Pramod Dematagoda
<pmd.lotr.gandalf@...> wrote:

> But the situation is this, this notification is being shown when GNOME
> starts loading, and in my laptop, this is when there is nothing visible
> in the screen at all apart from the wallpaper, so the notification gets
> attached to an icon which does not seem to exist at that time, which
> probably explains why the notification is just seen floating instead of
> showing up somewhere sensible. So this problem may not necessarily be
> something wrong with notification-daemon but with the issue that GPM
> shows the notification just too quickly to be attached to a present
> icon. If you could suggest what would be best in this case, I'll see
> what I can do. :)

Ohh, in that case we just need to add a delay to the icon showing up.
There are other notifications that do this already IIRC, just search
for g_timeout_add.

Richard.
_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, 2009-05-15 at 17:28 +0100, Richard Hughes wrote:

> On Fri, May 15, 2009 at 4:34 PM, Pramod Dematagoda
> <pmd.lotr.gandalf@...> wrote:
> > But the situation is this, this notification is being shown when GNOME
> > starts loading, and in my laptop, this is when there is nothing visible
> > in the screen at all apart from the wallpaper, so the notification gets
> > attached to an icon which does not seem to exist at that time, which
> > probably explains why the notification is just seen floating instead of
> > showing up somewhere sensible. So this problem may not necessarily be
> > something wrong with notification-daemon but with the issue that GPM
> > shows the notification just too quickly to be attached to a present
> > icon. If you could suggest what would be best in this case, I'll see
> > what I can do. :)
>
> Ohh, in that case we just need to add a delay to the icon showing up.
> There are other notifications that do this already IIRC, just search
> for g_timeout_add.
>
> Richard.

Hello Richard,

I have tried implementing what you suggested, but I am unsure as to
where exactly I ought to stuff the g_timeout_add to, when I find one
function to set the tray icon, I find another that seems to have some
part in the fray, a few that I've found:-

1) gpm_tray_icon_init - gpm-tray-icon.c : 615

2) gpm_notify_use_status_icon - gpm-notify.c : 209

so some guidance as to what to make of this would be really helpful. :)

Thanks.

Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 2009-05-16 at 16:34 +0530, Pramod Dematagoda wrote:

> On Fri, 2009-05-15 at 17:28 +0100, Richard Hughes wrote:
> > On Fri, May 15, 2009 at 4:34 PM, Pramod Dematagoda
> > <pmd.lotr.gandalf@...> wrote:
> > > But the situation is this, this notification is being shown when GNOME
> > > starts loading, and in my laptop, this is when there is nothing visible
> > > in the screen at all apart from the wallpaper, so the notification gets
> > > attached to an icon which does not seem to exist at that time, which
> > > probably explains why the notification is just seen floating instead of
> > > showing up somewhere sensible. So this problem may not necessarily be
> > > something wrong with notification-daemon but with the issue that GPM
> > > shows the notification just too quickly to be attached to a present
> > > icon. If you could suggest what would be best in this case, I'll see
> > > what I can do. :)
> >
> > Ohh, in that case we just need to add a delay to the icon showing up.
> > There are other notifications that do this already IIRC, just search
> > for g_timeout_add.
> >
> > Richard.
>
> Hello Richard,
>
> I have tried implementing what you suggested, but I am unsure as to
> where exactly I ought to stuff the g_timeout_add to, when I find one
> function to set the tray icon, I find another that seems to have some
> part in the fray, a few that I've found:-
>
> 1) gpm_tray_icon_init - gpm-tray-icon.c : 615
>
> 2) gpm_notify_use_status_icon - gpm-notify.c : 209
>
> so some guidance as to what to make of this would be really helpful. :)
>
> Thanks.
>
> Pramod Dematagoda
>
Please disregard my last email. I looked around a bit more this time and
I think I cracked it with this new patch which attaches the status icon
to notifications after a 10 second wait and which seems to work just as
expected on my laptop.

Pramod Dematagoda

[setup_status_icon_attachment_after_some_time.patch]

Index: gpm-tray-icon.c
===================================================================
--- gpm-tray-icon.c (revision 3399)
+++ gpm-tray-icon.c (working copy)
@@ -607,6 +607,20 @@
 }
 
 /**
+ * This attaches the notification to the status icon after a 10 second wait.
+ * This is only done once during the initialisation of GPM.
+ **/
+static gboolean
+gpm_tray_status_icon_handler(gpointer temp)
+{
+ GpmTrayIcon *icon = (GpmTrayIcon *) temp;
+
+ gpm_notify_use_status_icon (icon->priv->notify, icon->priv->status_icon);
+
+ return FALSE;
+}
+
+/**
  * gpm_tray_icon_init:
  *
  * Initialise the tray object
@@ -644,8 +658,13 @@
  "activate",
  G_CALLBACK (gpm_tray_icon_activate_cb),
  icon, 0);
- gpm_notify_use_status_icon (icon->priv->notify, icon->priv->status_icon);
 
+ /* Attach the status icon after a wait of 10 seconds, it allows for the GNOME session to have loaded properly
+ * before showing any notifications and making a hash of things.
+ */
+
+ g_timeout_add_seconds( 10, gpm_tray_status_icon_handler , icon );
+
  /* only show the suspend and hibernate icons if we can do the action,
    and the policy allows the actions in the menu */
  allowed_in_menu = gconf_client_get_bool (icon->priv->conf, GPM_CONF_UI_SHOW_ACTIONS_IN_MENU, NULL);


_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Richard Hughes-6 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, May 16, 2009 at 2:11 PM, Pramod Dematagoda
<pmd.lotr.gandalf@...> wrote:
> Please disregard my last email. I looked around a bit more this time and
> I think I cracked it with this new patch which attaches the status icon
> to notifications after a 10 second wait and which seems to work just as
> expected on my laptop.

Excellent, thanks. I've committed your patch with a few minor changes.
I've attached the final patch I committed for your review. Thanks.

Richard.


_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

0001-Attach-the-libnotify-popups-to-the-notification-icon.patch (2K) Download Attachment

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 2009-05-16 at 18:14 +0100, Richard Hughes wrote:

> On Sat, May 16, 2009 at 2:11 PM, Pramod Dematagoda
> <pmd.lotr.gandalf@...> wrote:
> > Please disregard my last email. I looked around a bit more this time and
> > I think I cracked it with this new patch which attaches the status icon
> > to notifications after a 10 second wait and which seems to work just as
> > expected on my laptop.
>
> Excellent, thanks. I've committed your patch with a few minor changes.
> I've attached the final patch I committed for your review. Thanks.
>

Thanks for accepting my patch(Pretty much the first technical patch I've
ever submitted in my life).:)

And thanks for the changes as well, I learned a little more about how to
program in the process of looking at it.:)

Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 2009-05-16 at 18:14 +0100, Richard Hughes wrote:

> On Sat, May 16, 2009 at 2:11 PM, Pramod Dematagoda
> <pmd.lotr.gandalf@...> wrote:
> > Please disregard my last email. I looked around a bit more this time and
> > I think I cracked it with this new patch which attaches the status icon
> > to notifications after a 10 second wait and which seems to work just as
> > expected on my laptop.
>
> Excellent, thanks. I've committed your patch with a few minor changes.
> I've attached the final patch I committed for your review. Thanks.
>
> Richard.

Sorry to bring this up again Richard, but it would seem that my patch
still hasn't reached trunk. I've updated my local copy(by accident, I
had used the svn branch, but I soon realised my error and switched to
the git branch(this ought to be mentioned on the GPM web site)). I hope
something wrong hadn't happened to the patch at the last minute?

Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

Re: [Patch] Remove the icon attachment when showing the battery capacity warning

by Bugzilla from pmd.lotr.gandalf@gmail.com :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, 2009-05-22 at 14:10 +0530, Pramod Dematagoda wrote:

> On Sat, 2009-05-16 at 18:14 +0100, Richard Hughes wrote:
> > On Sat, May 16, 2009 at 2:11 PM, Pramod Dematagoda
> > <pmd.lotr.gandalf@...> wrote:
> > > Please disregard my last email. I looked around a bit more this time and
> > > I think I cracked it with this new patch which attaches the status icon
> > > to notifications after a 10 second wait and which seems to work just as
> > > expected on my laptop.
> >
> > Excellent, thanks. I've committed your patch with a few minor changes.
> > I've attached the final patch I committed for your review. Thanks.
> >
> > Richard.
>
> Sorry to bring this up again Richard, but it would seem that my patch
> still hasn't reached trunk. I've updated my local copy(by accident, I
> had used the svn branch, but I soon realised my error and switched to
> the git branch(this ought to be mentioned on the GPM web site)). I hope
> something wrong hadn't happened to the patch at the last minute?
>
> Pramod Dematagoda
>
My apologies, the patch has come down at last. Please disregard my last
email. Thanks again Richard.:)

Pramod Dematagoda

_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list