managing of contacts in multiple groups is broken

Bug #507360 reported by buzzdee
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Coccinella
Fix Released
Medium
buzzdee

Bug Description

When having a contact in mulitple groups, e.g. ContactA in groupA and groupB.

When using RMB on a contact to edit contact, then both groups are shown in the list.
it is A) not possible with coccinella to add a contact to more than one group. Or is there a hidden way to do so, Sander?

and B) when removing a group (setting the value in the field to None or removing its contents)
           and clicking save, then the contact is removed from the online contacts and only shown in the respective groups below
           the offline contacts

And C) when someone changes the group assignments (adding the contact to other groups) with another client (e.g. tkabber) while also online with coccinella: The same as in B) happens

And D) in case a user is assigned to more than one group, it is possible to select the same group multiple times, without a warning or notice message. (however, only one group item is sent to the server in the roster update, so the duplicates get filtered out)

proposed solution for A) and D):
change the edit contact dialog to look like the one of tkabber, see appended screenshot.

proposed solution for B) and C):
fix it ;)

I should probably fix this, at least the problems shown in B) and C) before going on with #150360

Revision history for this message
buzzdee (sebastia) wrote :
Changed in coccinella:
assignee: nobody → buzzdee (sebastia)
status: New → In Progress
Revision history for this message
buzzdee (sebastia) wrote :
Download full text (4.4 KiB)

Problem B) and C) only seems to happens when I try to put myself in two groups.

I also get this exception message now, when I login:

preshook ::namespace inscope ::Roster PresenceEvent failed: 1
bad modifier "88": must be above, ancestors, below, bottom, child, children, descendants, firstchild, lastchild, left, leftmost, next, nextsibling, parent, prev, prevsibling, right, rightmost, sibling, or top
    while executing
"$T item element configure $item cTree $elem -image $image"
    (procedure "SetAltImage" line 29)
    invoked from within
"SetAltImage $jid $key $value"
    (procedure "::RosterPlain::SetItemAlternative" line 5)
    invoked from within
"$plugin($name,setItemAlt) $jid $key image $value"
    (procedure "StyleConfigureAltImages" line 8)
    invoked from within
"StyleConfigureAltImages $jid"
    (procedure "::RosterTree::StyleCreateItem" line 6)
    invoked from within
"::RosterTree::StyleCreateItem $jid "available" -name buzz -groups {Coccinella Cacher} -subscription none -resource Coccinella@sre -type available"
    ("eval" body line 1)
    invoked from within
"eval {
  ::RosterTree::StyleCreateItem $jid "available"
     } $itemAttr [array get presA]"
    ("highest-prio" arm line 15)
    invoked from within
"switch -- $config(roster,multi-resources) {

 "highest-prio" {

     # Add only the one with highest priority.
     set jid2 [jlib::barejid $jid]
   ..."
    (procedure "NewAvailableItem" line 11)
    invoked from within
"NewAvailableItem $rjid"
    (procedure "Presence" line 70)
    invoked from within
"Presence $jid3 $type -from <email address hidden>/Coccinella@sre -type available -resource Coccinella@sre -xmldata {presence {from sebastia@l00..."
    ("eval" body line 1)
    invoked from within
"eval {Presence $jid3 $type} $opts"
    (procedure "PresenceEvent" line 63)
    invoked from within
"PresenceEvent ::jlib::jlib1 {presence {from <email address hidden>/Coccinella@sre xml:lang en to <email address hidden>/Coccinella@sre} ..."
    (in namespace inscope "::Roster" script line 1)
    invoked from within
"::namespace inscope ::Roster PresenceEvent ::jlib::jlib1 {presence {from <email address hidden>/Coccinella@sre xml:lang en to sebastia@l00-bugd..."
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $func [list $jlibname $xmldata]"
bad modifier "88": must be above, ancestors, below, bottom, child, children, descendants, firstchild, lastchild, left, leftmost, next, nextsibling, parent, prev, prevsibling, right, rightmost, sibling, or top
    while executing
"$T item element configure $item cTree $elem -image $image"
    (procedure "SetAltImage" line 29)
    invoked from within
"SetAltImage $jid $key $value"
    (procedure "::RosterPlain::SetItemAlternative" line 5)
    invoked from within
"$plugin($name,setItemAlt) $jid $key image $value"
    (procedure "StyleConfigureAltImages" line 8)
    invoked from within
"StyleConfigureAltImages $jid"
    (procedure "::RosterTree::StyleCreateItem" line 6)
    invoked from within
"::RosterTree::StyleCreateItem $jid "available" -name buzz -groups {Coccinella Cacher} -subscription none -resource Coccinella@sre -type available"
    ("eval" b...

Read more...

Revision history for this message
buzzdee (sebastia) wrote :

Problem B) and C) only seems to happens when I try to put myself in two groups, and with ejabberd. I installed openfire, and there I was easily able to add myself to the roster.

Openend a bug report:
https://support.process-one.net/browse/EJAB-1166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=37612#action_37612

As pointed out there, probably the solution is to add a function like tkabber has: show_own_resources.

Revision history for this message
buzzdee (sebastia) wrote :

ejabberd will probably change behaviour to return a not-allowed when trying to add the own JID as contact to the roster.

tkabber has an option to show the own resources in the roster, we should probably do the same.

Revision history for this message
buzzdee (sebastia) wrote :

The proposed solution for A) and D) is implemented with svn revision: 2780.
However, the roster only seems to handle it nicely with the RosterTwo lines style.

In the plain style, I get above menitioned exception.

The styles defined in RosterAvatar.tcl, show the contact only in the first group in the group list.

Revision history for this message
buzzdee (sebastia) wrote :

svn revision 2781 contains the fix for the exception.
So the Roster Plain and Two lines are fine now.
The Styles defined in RosterAvatar.tcl need to be reworked completely.
The problem there is that groups and jids are all put in the tree below root, not like the other two, the jids are not put below the groups.

Revision history for this message
sander (s-devrieze) wrote :

Some visual feedback:
* Alignment of the window elements. For instance, "Available groups" should be aligned similar as "Nickname". The same is true for all other elements below the spacer, including the buttons.
* What about hiding the whole groups thing behind a "More..." button (like e.g. in the Login dialog)? Maybe the whole contact actions feature also can be moved here to a tab, in addition to the annotations? Or maybe the whole "Edit Contact" dialog should consist of the tabs General, Groups, Actions, and Annotations. I am not sure what will be the most userfriendly...?
* It should be possible to "copy" a contact to another group by dragging and dropping the contact whilst holding ctrl.
* Also, when a contact with multiple groups is *moved* to another group, the group from which it is moved should be replaced by the new group. This contact should still stay in the other groups.

Revision history for this message
sander (s-devrieze) wrote :

"tkabber has an option to show the own resources in the roster, we should probably do the same."

Yes, but I think this should be a hidden feature (key combination to show/hide this "fake contact").

Revision history for this message
sander (s-devrieze) wrote :

btw, is it possible the whole feature is broken? It also does not currently work using the roster two lines style...

Revision history for this message
buzzdee (sebastia) wrote :

regarding comment #7: you mean more or less merge the Edit contact dialog with the View business card?
Or what I then propose:
merge the Edit Contact, with Contact Actions, and the Notes from the View Business Card into a single dialog.
Then having four tabs in the dialog:
* General
     * nickname configuration
     * subscriptions
* Groups
     * managing the groups
* Notes
     * just the notes
* Contact Actions
     * managing the contact actions, integrating the actual tabs in a subframe of the tab

When the notes are removed from the View business card, then the "save" button could be removed there too.
Also the notes could also stay there, and may just be available in two places afterwards.

regarding comment #8: I'm fine with your proposal, but maybe also "dumb" uses may have the idea to send a message to another online resource of their own. Whereas the key combination may be too tricky (also if we add a hint to the TOTD)

regarding comment #9: It works for me with the Two lines style, what problem do you have there?

Revision history for this message
buzzdee (sebastia) wrote :

again regarding comment #7: I did not tried yet how the dnd works, but I think you proposal should be the way to go

Revision history for this message
sander (s-devrieze) wrote : Re: [Bug 507360] Re: managing of contacts in multiple groups is broken

> Then having four tabs in the dialog:
> * General
>     * nickname configuration
>     * subscriptions
> * Groups
>     * managing the groups
> * Notes
>     * just the notes
> * Contact Actions
>     * managing the contact actions, integrating the actual tabs in a subframe of the tab

+1. There is one thing that needs to be solved though: how to set
contact actions for a whole group instead of 1 individual contact? Do
you think this is advanced functionality that should be dropped to
make Coccinella more userfriendly?

> When the notes are removed from the View business card, then the "save" button could be removed there too.

+1. However, I think there should be a link in the business card
dialog to open the Notes tab of the Edit Contact dialog. The View
Business Card dialog then becomes read-only which is more natural for
a "View" dialog.

> Also the notes could also stay there, and may just be available in two places afterwards.

+1

> regarding comment #8: I'm fine with your proposal, but maybe also "dumb"
> uses may have the idea to send a message to another online resource of
> their own. Whereas the key combination may be too tricky (also if we add
> a hint to the TOTD)

A key combination is enough. It will be still possible to add a GUI to
enable this later, but I don't think this will be needed; I've never
seen questions like this in the forum.

> regarding comment #9: It works for me with the Two lines style, what
> problem do you have there?

If I add a contact to multiple groups and save the edits, the new
groups appear in my roster *without* the contact in it. If I relogin,
all groups are gone, but if I open the edit contact dialog it shows
the current groups correctly.

Revision history for this message
buzzdee (sebastia) wrote :

regarding comment #7:

> * It should be possible to "copy" a contact to another group by dragging and dropping the contact whilst holding ctrl.
After checking how this is done in coccinella, found that this dnd of contacts within the roster is handled by tktreectrl.

$T notify install <Drag-receive>
    $T notify bind $T <Drag-receive> {
        ::RosterTree::NotifyDragReceive %W %l %I
    }

Reading the manual page of tktreectrl, there is no way to tell the ::RosterTree::NotifyDragReceive command, (at least there is no % thingie that can tell the receiving function whether a control key was used or not) whether a control key was pressed while dragging.

As a workaround, in NotifyDragReceive we could ask the user whether he would like to copy or move the contact, and then act as he wishes.

tkDND has a %m, that can tell the receiving function, whether a control key, and especially which one was used, and then the receiving function could act accordingly. Maybe later switching to tkDND for that?

Revision history for this message
buzzdee (sebastia) wrote :

As said in the comment before, I don't see an easy way to use a key combination for the DnD, and that still did not changed ;).

Appended patch now adds a popup menu, asking the user whether to copy or move the contact to the other group, and can also cancel the action.
The function handling the DnD, is now also aware of the fact that a user can be a member in multiple groups, which it was not before.

It is still added in a very hackish way, I still need to change a little bit. I already have an idea what to do, probably on the weekend... However, the "look n feel" will not change, only how the goal is achieved.

The only glitch right now is, that after the popup menu is shown, it may be below the balloon of the user.

I'd like to make the appearance of the balloon dependent on the existence of the popup window, still need to find out where the balloon gets created.

Sander, any comments on this DnD part? At least for me the best compromise right now.

Revision history for this message
buzzdee (sebastia) wrote :

While on the train on the weekend, I had some more time to take a look at the DnD of the contacts in the roster.
I made it aware of the fact that a contact can be a member of multiple groups.
Implemented in a not so hackish way in RosterTree.tcl than the patch I added here some days ago.
Further, in the styles defined in RosterAvatar.tcl. and in RosterTwo.tcl, DnD was not enabled. It is now also enabled.
svn revision 2784 contains the changes.
However, the styles defined in RosterAvatar.tcl are only showing the contacts in one of the group a contact is a member of. The activated DnD is doing the right thing, copying or moving.

However, needs probably testing, feedback welcome.

Revision history for this message
buzzdee (sebastia) wrote :

I just recognized, using the Plain or Two lines style, then the number of offline/online contacts is wrong.
In case there is a contact in more than one group, then the contact is counted multiple times.
The other styles defined in RosterAvatar.tcl do not have the problem, because they do not have such "head" groups.

Revision history for this message
sander (s-devrieze) wrote :

>> * It should be possible to "copy" a contact to another group by dragging and dropping the contact whilst holding ctrl.
> After checking how this is done in coccinella, found that this dnd of contacts within the roster is handled by tktreectrl.
<snip>
> tkDND has a %m, that can tell the receiving function, whether a control
> key, and especially which one was used, and then the receiving function
> could act accordingly. Maybe later switching to tkDND for that?

Yes. It may be that Mats did not used tkDND because it did not
supported Mac OS X. The new 2.0 version seems to support Mac OS X. So
it now may be a good idea to switch to 2.0 and use tkDND for the
above.

Revision history for this message
sander (s-devrieze) wrote :

> While on the train on the weekend, I had some more time to take a look at the DnD of the contacts in the roster.
> I made it aware of the fact that a contact can be a member of multiple groups.
> Implemented in a not so hackish way in RosterTree.tcl than the patch I added here some days ago.
> Further, in the styles defined in RosterAvatar.tcl. and in RosterTwo.tcl, DnD was not enabled. It is now also enabled.
> svn revision 2784 contains the changes.
> However, the styles defined in RosterAvatar.tcl are only showing the contacts in one of the group a contact is a member of. The activated DnD is doing the right thing, copying or moving.
>
> However, needs probably testing, feedback welcome.

Looks good. Some feedback:
* Capitalization
* Key short cuts (use the same ones as in KDE if such a menu is still
available in KDE 4)

Revision history for this message
sander (s-devrieze) wrote :

"> regarding comment #9: It works for me with the Two lines style, what
> problem do you have there?

If I add a contact to multiple groups and save the edits, the new
groups appear in my roster *without* the contact in it. If I relogin,
all groups are gone, but if I open the edit contact dialog it shows
the current groups correctly."

Unfortunately, this bug is still available in all roster styles. This is very annoying.

Revision history for this message
sander (s-devrieze) wrote :

The option "Show all online clients of a contact" option also causes issues if enabled. I don't think this is related to the dnd but it may be.

Revision history for this message
sander (s-devrieze) wrote :

"actually do you can try to reproduce this error:
scroll down in the roster, click on the last entry so that it gets marked, and then press ESC"

I get the same error on Linux.

Revision history for this message
buzzdee (sebastia) wrote :

regarding comment #18
I switched to capitalization and added Keyboard shortcuts to the popup menu, how useful they may be at all, the user is doing DnD with the mouse, don't know really what the reason could be to use keyboard shortcuts in the menu ;)

Revision history for this message
buzzdee (sebastia) wrote :

regarding comment #19 and #20, do you have a clean checkout of the sources or a binary from the breakfast service for testing?
I am still unable to reproduce the problem mentioned in comment #19, in case you have patches applied from bug #150360, they could potentially cause problems, but there is nothing from that in svn. I had too much trouble to get this clean implemented and reliably to work.

If not, could you start coccinella with a fresh configuration, and diff the new and the old configuration?

Revision history for this message
buzzdee (sebastia) wrote :

This one is mostly fixed, Sander, I hope it also works for you?

I close this, added a new follow up on the styles that show the contact only once.

With regard to changing the edit contact UI, e.g. tabs ...
Sander, could you create a new bug report if you still would like to have it?

Changed in coccinella:
status: In Progress → Fix Committed
Revision history for this message
sander (s-devrieze) wrote :

It is as you said in comment #23, the patches of bug #150360 were causing the trouble. It works great now, although there still are two small issues:
1) If you try to copy a contact from a group to the "online root", a copy does not appear. I guess this is due to the protocol, right? If so, I'll report this the the people who wrote the protocol.
2) If you have your own JID in the roster and you copy this to a group, it moves to the offline group. If you reconnect, it appears in the group.

As these are small issues, I guess it's better to look at them after the release.

Revision history for this message
buzzdee (sebastia) wrote :

thanks for retesting,
regarding comment 1) Yes, I think it is how the protocol works, as far as I can see, I cannot have a contact in a group, and outside of all groups in parallel

regarding comment 2)
I have a bug open with ejabberd:
https://support.process-one.net/browse/EJAB-1166
RFC3921bis says a user should not been allowed to add himself to the roster
I'll see that I get the thing with the own JID in the roster done with the next release...

Revision history for this message
sander (s-devrieze) wrote :

1) I just discussed this in <email address hidden>. See the logs when they are online again.

buzzdee (sebastia)
Changed in coccinella:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.