Opened 12 years ago

Closed 10 years ago

#190 closed patch (fixed)

single window for buddy pounces

Reported by: bjlockie Owned by: kstange
Milestone: 2.6.0 Component: pidgin (gtk)
Version: 2.0 Keywords:
Cc: phannent

Description (last modified by kstange)

There should only be one buddy pounce window showing at one time. The window should open with the first buddy pounce and subsequent buddy pounces can be added to a list that is displayed in the window.

It is annoying when I go away and come back to my computer to see millions of buddy pounce windows. :-)

Attachments (6)

pounce_dialog.patch (13.9 KB) - added by salinasv 11 years ago.
Apply this first.
cosmetic.patch (5.4 KB) - added by salinasv 11 years ago.
Little fix
all_patch.diff (18.6 KB) - added by bjlockie 11 years ago.
a diff
set_null.patch (600 bytes) - added by salinasv 11 years ago.
This must be on top of my first two patches
dialog_dismiss_edit.patch (19.1 KB) - added by salinasv 10 years ago.
new patch fixing stuff
pounce_dialog-22-02-09.patch (22.9 KB) - added by salinasv 10 years ago.
Forgot to commit a cleanup.

Download all attachments as: .zip

Change History (42)

comment:1 Changed 12 years ago by salinasv

This could be equal to the mail notification window with a scrolled window and a treeview inside

I'm also thinking that this is a common used function in Pidgin... maybe could be a pidgin_util stuff ;)

comment:2 Changed 12 years ago by lschiere

  • Owner set to seanegan

Sean, this is similar to other changes you have made.

comment:3 Changed 11 years ago by oa

Shouldn't buddy pounces really be using libnotify to provide an unobtrusive notification bubble?

comment:4 Changed 11 years ago by deryni

Having buddy pounces optionally use libnotify for notifications would be fine, but depending on that would break a feature that used to work if people build without it. That being said buddy pounce dialogs are long-lived while libnotify notifications are transient (as far as I know anyway) so they really aren't an accurate replacement. Using buddy pounce dialogs for transient notifications of things isn't what there were designed for, use the pidgin-libnotify plugin or the guifications plugin for that stuff.

comment:5 Changed 11 years ago by bjlockie

I have had >50 dialogs when I get back if I leave pidgin running when I go out.

comment:6 Changed 11 years ago by salinasv

  • Owner changed from seanegan to salinasv
  • Status changed from new to assigned

As I said before, this can be done using the mail notification code.

I will try to get this working this days. If someone have an objections, let me know.

comment:7 Changed 11 years ago by bjlockie

bump :-)

Changed 11 years ago by salinasv

Apply this first.

comment:8 Changed 11 years ago by salinasv

  • Type changed from enhancement to patch

Finally opened vim and start coding this and upload a patch. It looks like doesn't break anything. It will force a 2.6

Can someone take a look?

Changed 11 years ago by salinasv

Little fix

comment:9 Changed 11 years ago by bjlockie

I am testing it on a 2.5.1 install (kbuntu) and I will also patch my 2.5.1 (gentoo).

Great work, it is exactly what I wanted.

comment:10 Changed 11 years ago by bjlockie

I closed the buddy pounce window and I can't get it back.

comment:11 Changed 11 years ago by salinasv

The dialog is not persistent. When you close the dialog is because you have already noticed the pounces => there are no longer needed.

comment:12 Changed 11 years ago by bjlockie

The problem is new pounces don't open a window. I'll try it on my other computer.:-)

comment:13 Changed 11 years ago by bjlockie

I created a recurring buddy pounce and my friend logged off and on twice with the pounce window open. I then closed the window and he logged off and on and the window did NOT open.

comment:14 Changed 11 years ago by bjlockie

I don't know how to create a patch but I added this:

"gtknotify.c" line 1323 of 1403

make sure window is opened gtk_dialog_run( spec_dialog );

It seems to work.

comment:15 Changed 11 years ago by bjlockie

// make sure window is opened
gtk_dialog_run( spec_dialog );

comment:16 Changed 11 years ago by bjlockie

I was getting some GTK_IS_WIDGET errors so I modified the code. I'll diff it from the released source and post the diff.

What do you use for testing for memory leaks?

Changed 11 years ago by bjlockie

a diff

comment:17 Changed 11 years ago by bjlockie

My diff SHOULD be the modified patch and the cosmetic patch in one file. It works on my two wildy different computers.

Changed 11 years ago by salinasv

This must be on top of my first two patches

comment:18 Changed 11 years ago by salinasv

I have uploaded a correct fix.

The real problem was caused because I was waiting a GTK_RESPONSE_CLOSE to free the data and set the dialgo to NULL to know we have closed it.

This new patch looks like working and fix the leakage from never freeing that model tree.

comment:19 Changed 11 years ago by bjlockie

Works fine on my Gentoo and Ubuntu computers. Can you get the patches in the main tree? I'm sure people will like this (or at least not mind :-)).

comment:20 Changed 11 years ago by rekkanoryo

  • Owner changed from salinasv to kstange

Kevin, would you review this patch and either apply or reject?

comment:21 follow-up: Changed 10 years ago by kstange

There are a few different problems with this patch that I see:

1) I think that individual items, being possible to select within the treeview, ought to have some function, or else selection should not be possible. I would suggest these things (as possibilities):

a) A dismiss button to remove an item from the list

b) An Edit pounce button if the pounce that was triggered is recurring

c) A Get Info or New Message button for buddies that are online.

d) Focus the newest treeview item and scroll to it when it appears.

2) The messages are inconsistent. They're sentence fragments now, which is easily enough fixed, but they should be simple statements describing what happened, since the dialog does not form actual sentences.

3) When you request a popup notification, there is a field for a message, which formerly appeared in the dialog box that would pop up. This is missing from the new window.

comment:22 Changed 10 years ago by rekkanoryo

  • Milestone set to Patches Needing Improvement

From Kevin's comment, I take it that this patch needs work.

comment:23 Changed 10 years ago by bjlockie

I really hope salinasv fixes up the patch because I think it is way better. :-)

Changed 10 years ago by salinasv

new patch fixing stuff

comment:24 in reply to: ↑ 21 ; follow-up: Changed 10 years ago by salinasv

Just attached a new patch wich fix some stuff and implements a dismiss and edit button.

Replying to kstange:

There are a few different problems with this patch that I see:

1) I think that individual items, being possible to select within the treeview, ought to have some function, or else selection should not be possible. I would suggest these things (as possibilities):

I haven't added more buttons because they will not fit in the dialog and looks ugly. If someone want to add more actions I guess the best bet will be a context menu (stuff that I don't know how to do)

d) Focus the newest treeview item and scroll to it when it appears.

Didn't found a way to do this one.

2) The messages are inconsistent. They're sentence fragments now, which is easily enough fixed, but they should be simple statements describing what happened, since the dialog does not form actual sentences.

I hope the commiter can fix this strings.

3) When you request a popup notification, there is a field for a message, which formerly appeared in the dialog box that would pop up. This is missing from the new window.

What do you mean?

comment:25 in reply to: ↑ 24 ; follow-up: Changed 10 years ago by kstange

Replying to salinasv:

3) When you request a popup notification, there is a field for a message, which formerly appeared in the dialog box that would pop up. This is missing from the new window.

What do you mean?

When you create a pounce, there is an option for "pop up a notification" which has a text entry field next to it. Any text you enter in that field is displayed in the dialog box. If I enter text in the box with your patch it is unused.

comment:26 in reply to: ↑ 25 Changed 10 years ago by salinasv

Replying to kstange:

Replying to salinasv:

3) When you request a popup notification, there is a field for a message, which formerly appeared in the dialog box that would pop up. This is missing from the new window.

What do you mean?

When you create a pounce, there is an option for "pop up a notification" which has a text entry field next to it. Any text you enter in that field is displayed in the dialog box. If I enter text in the box with your patch it is unused.

It is fixed in the last patch. I don't know why I missed it the first time.

comment:27 follow-up: Changed 10 years ago by bjlockie

Which patches do I need? I put: dialog_dismiss_edit.patch on 2.5.3 and the single window is there but the dismiss and edit buttons don't work.

comment:28 in reply to: ↑ 27 Changed 10 years ago by salinasv

Replying to bjlockie:

Which patches do I need? I put: dialog_dismiss_edit.patch on 2.5.3 and the single window is there but the dismiss and edit buttons don't work.

That's the correct patch to use. It's working for me, can you elaborate what do you see?

You may want to add comments in pounce_response_cb() pounce_responce_edit_cb() and pounce_response_dismiss() to see what's going on.

comment:29 Changed 10 years ago by bjlockie

The buttons are there but nothing happens. I'll recompile it, it is probably this one computer.

comment:30 Changed 10 years ago by bjlockie

  • Description modified (diff)

Figured it out, user error. :-) I wasn't highlighting the pounce to edit or dismiss so it wasn't doing anything.

comment:31 Changed 10 years ago by kstange

  • Description modified (diff)

comment:32 Changed 10 years ago by kstange

  • Milestone changed from Patches Needing Improvement to 2.6.0

Just to update you on this, I would like to get this in Pidgin 2.6.0, but I need to review the code, and do some extra QA and testing. I have a few thoughts for tweaks in my head, particularly setting the urgent hint on this window when it has an item added to it, and sensitizing buttons appropriately based on the pounce entry selected. I believe I should be able to do the scroll-to-target as new items appear.

Also, now that the patch honors the custom message, it no longer tells you why the message occurred, which I think needs to be resolved, because it's possible for the same pounce to be triggered by several different events, meaning the message doesn't tell you much on its own.

I don't see any reason (based on the size of the dialog) that there's a problem with more than 3 buttons. 5 should fit comfortably, though that's probably more than are needed.

Also, there's an obvious crash for a pounce that is deleted or goes away between appearing and clicking the Edit button.

Any such improvements are welcome while I find some free time and mull stuff in my head. :)

comment:33 Changed 10 years ago by kstange

  • Milestone changed from 2.6.0 to Patches Needing Improvement

Let's keep this as PNI until I've gotten it resolved. Can always delay to 2.7.0 if I need to.

comment:34 Changed 10 years ago by salinasv

I have just uploaded a new patch that fixes the crash when the pounce is deleted and the user click the edit button. It also sensitize the buttons appropriately and display both, the event that triggered the pounce and the specified message.

Changed 10 years ago by salinasv

Forgot to commit a cleanup.

comment:35 Changed 10 years ago by kstange

  • Milestone changed from Patches Needing Improvement to 2.6.0

Just to let you know, the patch looks pretty good. I finally got back around to testing it. I am tweaking and testing a few minor things and hopefully I will be able to commit it soon.

comment:36 Changed 10 years ago by kstange

  • Resolution set to fixed
  • Status changed from new to closed

I committed the patch. I referenced this ticket number but it doesn't seem to have worked, so closing manually.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!