Several places of the candy codebase have too much lines of code per function and also too much logic in it. Also, we should reconsider the rooms/users memory "storage". As far as I know, we have e.g. room informations three times stored: In core, in view and in the DOM. This should not be the case as it adds unnecessary overhead.

This issue should list potential parts to be refactored and how to do it (I'll update this post with the findings based on current dev branch). If possible, we can also enhance User/Contributor UX by simplifying things.

To do these refactorings, we should create a new branch based on dev and apply them there (also maybe for bigger refactorings, create a branch on top of the refactoring branch).

Too much logic

• [x] Various escaping & unescaping on core/(action|event) and view/*
  • It should be: unescape in core/event & escape in core/action - nowhere else one should care about it
• [x] Candy.Core.Event.Jabber.Room.Presence is way too much code & partly responsible for the mess in other presence handling code (of the view)
• [ ] Candy.Core.Event.Jabber.Room.Message - too much code, split up in well named functions
• [x] Candy.View.Observer.Presence.update - too much logic, split up in well named functions
• [ ] Candy.View.Pane.PrivateRoom.changeNick - too much logic, too much dom changes etc.
• [ ] Candy.View.Pane.Roster.update - oh boy - simplify this! Simplifying other presence related handling code first might help
• [ ] Candy.View.Pane.Roster.showJoinAnimation - too much logic for such a simple task
• [ ] Candy.View.Pane.Message.show - too much logic?
• [x] is current user checks in presence handlers - replace by checking for status code 110

UX enhancements

core.js

• [x] connect() calls: too much possibilities
  • Do we create multiple connectX() functions?
  • Or one connect() function which takes a param object?
• [ ] _user from the variable naming it's somehow unclear that this holds the current user - rename?

non-DRY

core.js

• [x] _getEscapedJidFromJid() duplicates functionality of Candy.Util.escapeJid()
• [ ] _rooms and it's functions: duplicates storage of view (Candy.View.Pane.Chat.rooms) & DOM (data attributes)

view/pane.js

• [ ] PrivateRoom vs. Room handling - merge into one with specific functionality if required?

Various things

• [ ] core.js registerEventHandlers() - are all of them really required to be defined there?
  • e.g. Bookmarks could be only registered before sending the iq query in Candy.Core.Action.Jabber.Autojoin()
• [x] is the strophe.muc.js plugin needed? Some of the things we do anyway already on our own, why not just remove the plugin and build the remaining stanzas on our own? Reduces size of Candy
• [ ] chatUser, chatRoom, chatRoster -- maybe improve those models, e.g. chatUser has nick and jid but those could maybe be merged
• [x] Split translations in separate files
• [ ] provide a way to know which strings are missing in which translation
• [ ] Candy.View.Pane.Chat.infoMessage vs. Candy.View.Pane.Chat.onInfoMessage- provide better naming to make it clear which function should be called and which not
• [ ] Update mustache.js
• [ ] Use bower for dependencies instead of submodules (if possible)

Conclusion

We have to do some stuff, but it's not that bad. After doing this refactoring, we might be able to finally do automatic unit tests.

About connect():

Let's create a function which takes a single parameter. Optionally it could handle/transform the params we're using right now for compatibility reasons.

Let's create a function which takes a single parameter. Optionally it could handle/transform the params we're using right now for compatibility reasons.

:+1:

connect(): how to do it?

5 possibilities, atm (from the docs):

• Candy.Core.connect('jid', 'password') -- Connnect as a registered user.
• Candy.Core.connect('servername') -- Connnect anonymously to a specific server. Users will be greeted with a nickname form.
• Candy.Core.connect('servername', null, 'nickname') -- Connect anonymously to a specific server, but don't greet the users with a login form. Instead the specified nickname will be used. The second param (password) has to be null, because anonymous logins don't have a password (of course).
• Candy.Core.connect('jid') -- Users will be greeted with a login form. In order to authenticate they have to provide their password.
• Candy.Core.connect() -- Users will be greeted with a login form. In order to authenticate they have to provide their JID ([email protected]) and password.

Proposal:

// connect as registered user
Candy.Core.connect({
  jid: '[email protected]',
  password: 'some-highly-secure-password-hopefully'
});

// connect anonymously to server
Candy.Core.connect({
  hostname: 'xmpp.example.com'
});

// connect anonymous, with nickname set
Candy.Core.connect({
  hostname: 'xmpp.example.com',
  nickname: 'test'
});

// connect with jid, but user has to enter pw
Candy.Core.connect({
  jid: '[email protected]',
});

// just show loginform
Candy.Core.connect();

What do you think? Still a bit complex, but better.

:+1:

I had the exact same code stashed in my local repo a couple of weeks ago (including support for the argument order of the previous version) ... gonna push this at will.

Maybe we could even write an adapter function in Candy (so that Candy.connect() just calls candy.core.connect()). Benefit: user doesn't need to know anything about the 'core'

On Fri, Jan 17, 2014 at 8:29 PM, Patrick Stadler [email protected] wrote:

:+1:

I had the exact same code stashed in my local repo a couple of weeks ago (including support for the argument order of the previous version) ... gonna push this at will.

Reply to this email directly or view it on GitHub: https://github.com/candy-chat/candy/issues/207#issuecomment-32638595

One more thing:

Candy.Core.connect({
    attach: {
        jid: '[email protected]',
        sid: '56da96be5419c52cbcf95594f129d537',
        rid: 1337
    }
});

Yes or no?

Candy.connect(); would make much more sense indeed.

Mhh. Unsure. Thought about it too but i think we should leave it away. Or if we want to do it, then move the jid (just the jid) param a level up (so that it aligns with the other connect methods which use a jid)

On Fri, Jan 17, 2014 at 8:34 PM, Patrick Stadler [email protected] wrote:

One more thing:

Candy.Core.connect({
    attach: {
        jid: '[email protected]',
        sid: '56da96be5419c52cbcf95594f129d537',
        rid: 1337
    }
});

Yes or no?

Reply to this email directly or view it on GitHub: https://github.com/candy-chat/candy/issues/207#issuecomment-32639014

(About pushing it: do it, i didnt work on it yet)

Same here, I don't know if attach makes sense, but I'd not like it to be mixed (jid on the level above). Maybe we simply provide Candy.attach(jid, sid, rid); and that's it then.

Ok.

On Fri, Jan 17, 2014 at 8:43 PM, Patrick Stadler [email protected] wrote:

Same here, I don't know if attach makes sense, but I'd not like it to be mixed (jid on the level above). Maybe we simply provide Candy.attach(jid, sid, rid); and that's it then.

Reply to this email directly or view it on GitHub: https://github.com/candy-chat/candy/issues/207#issuecomment-32639822

I'm glad you are working on these! I managed to use your code and it appeared that it smelled a lot, mainly because of a broken MVC paradigm, with multiple storage points of data (redundant or not) and even DOM-stored data. And also because it seems like you manage things badly, that are already and correctly handled by Strophe. By the way, you mentioned strophe.muc.js plugin in your list. It would be better to improve this plugin than to rewrite it, no? While you are refactoring things, I recommend you to forget about the fixed "room@domain/nick" way to identify users. This is a "through MUC" anonymous JID that only makes sense on [semi-]anonymous rooms. If you (or a contributor) plan to extend Candy with standard IM behavior, or to play well with non-anonymous rooms, you should manage a roster of real user JIDs. You also should stick better with the protocol, especially using status codes and handling errors instead of your own checks. And finally always permit to provide a callback (mainly to manage errors) - and manage errors (implementing Strophe.log when debug=true is a good start). Providing callbacks will also help implementing unit tests. I'm currently working on the old release of Candychat and have already fixed some of these issues I mention, but using Hacks, not refactoring. So again I'm glad you are working on it. I'm fully open to help you in this task (actually I've no spare time to code, but at least I can elaborate on the issues I mention).

I'm glad you are working on these! I managed to use your code and it appeared that it smelled a lot, mainly because of a broken MVC paradigm, with multiple storage points of data (redundant or not) and even DOM-stored data

Absolutely agree. I work gradually on improving things and want to have a sane MVC implementation which has a real model layer. Whether this will be in the DOM or not I'll have to look. First I have to clean it up a bit and afterwards it will be hopefully easier to make it sane.

And also because it seems like you manage things badly, that are already and correctly handled by Strophe.

Could you elaborate what we manage badly which is handled correctly by Strophe?

By the way, you mentioned strophe.muc.js plugin in your list. It would be better to improve this plugin than to rewrite it, no?

I'd like to improve it, I opened two PRs (strophe/strophejs-plugins#14, strophe/strophejs-plugins#15) but it doesn't seem anyone has the time to merge them in. Also, I don't agree with the general direction of the strophejs-plugins to be more and more written in CoffeeScript (I don't mind if anyone wants to write their application in CS, I just think it's bad for the JS ecosystem to have libraries/plugins written originally in CS).

About rewriting the muc plugin: I don't want to rewrite it, I just noticed that we have ~2 or ~3 calls to it and I don't think it's worth to use it for those simple calls. And last but not least: it also seems to store some rooms etc (I didn't really take a close look, I've just seen it when I was looking at how I can remove it). I'm not sure if this should be a task of the muc plugin or not.

While you are refactoring things, I recommend you to forget about the fixed "room@domain/nick" way to identify users. This is a "through MUC" anonymous JID that only makes sense on [semi-]anonymous rooms. If you (or a contributor) plan to extend Candy with standard IM behavior, or to play well with non-anonymous rooms, you should manage a roster of real user JIDs.

When we initially developed Candy, we were having this discussion about whether to use the MUC JID or the real JID for the roster and especially for private chats. We decided to use MUC JIDs because: a) Candy is a MUC client b) Determining the online status of a user you're having a private conversation with is harder because AFAIR you don't get presence updates unless you ask the participant for it (-> adding him to your roster).

Of course using MUC JIDs has the disadvantage that you can have two private rooms with the same user if you both are in two rooms. But IMHO this is a tradeoff I can live with.

Also: users with native clients can still open private conversations with users within Candy, we handle this as well (although currently not in a very beautiful way, code-wise.

You also should stick better with the protocol, especially using status codes and handling errors instead of your own checks.

Can you elaborate on this as well?

And finally always permit to provide a callback (mainly to manage errors) - and manage errors (implementing Strophe.log when debug=true is a good start). Providing callbacks will also help implementing unit tests.

Where would you like to have callbacks? Regarding Strophe.log(): Yes this might be something we could implement. I didn't see the necessity yet though.

I'm currently working on the old release of Candychat and have already fixed some of these issues I mention, but using Hacks, not refactoring.

I checked your fork, didn't see your changes. I suppose you didn't push them back then?

So again I'm glad you are working on it. I'm fully open to help you in this task (actually I've no spare time to code, but at least I can elaborate on the issues I mention).

Sure, you're more than welcome to help, even if it's not coding but just pointing out issues.

All in all, from your comment it seems to me that you're implementing something specific using Candy. I'd love to know what you're building, if possible :)

In effect, I haven't pushed it yet.

My project is to use XMPP as an in-game chat, but letting the choice of using an external client (mobile, desktop) up to the user. I'm planning to sync all instances of an user using Message Carbon and Prosody (because it allows the same user to have multiple resources on a MUC over an unique nick). And to sync MUC presence I will implement some custom presence stanzas. I want the users to be non-anonymous, so all my rooms are non-anonymous by default. I really want private IM through bare JID instead of through anonymous JID, and I really want to get rid of the issue you mentioned: if you start private chat with the same user from two different rooms, you will have two conversations. That's a tradeoff I cannot live with ;) (So I always use the bare JID sent in presence >x>item[jid] when available).

And last but not least: it also seems to store some rooms etc

That's why I'm asking why you don't rely on it to manage rooms. I didn't dig into this plugin, but since Strophe seems quite bug-free (I corrected one), I imagine that it does this well, I suppose..

About relying on status codes, for instance I saw at some places that you did Strophe.getResourceFromJid(from) === Candy.Core.getUser().getNick() to check for self-presence stanzas. But you have presence >x>status[code=110] for that.

Need to go to sleep now but I'll continue soon ;)

I'm back. Another important flaw is that Candy.Core.getUser().getNick() is unique through the application. But unlike other protocols, XMPP allow to have different nicks on different rooms. Even if you want to keep the same nick over all rooms, you technically can't ensure that if 1) there is a nick policy, 2) there is a nick conflict, 3) this nick is reserved, 4) the user is using an external XMPP client that allow per-room nick change (i.e. near all clients). These edge cases confirm the bad idea behind using this nick instead of code 110 to identify myself.

About that, a nick change is a presence stanza owning the status code 303. And a nick changed by service policy has a status code of 210. (Actually a nick change from A to B is handled as "A leaved the room", "B entered the room").

I reviewed more carefully the strophe muc plugin. It seems to handle everything provided by MUC, including nick change and non-anonymous rooms, and provide some handlers and callbacks. I admit it is not so well written than I expected, but it's a really good start to me. Even more than a start, it's actually pretty complete IMHO.

Which brings me to the callback topic. That's nice to be able to provide a callback when we send a stanza. For instance strophe.muc provide callbacks (success and error) for an info#disco on a room, and when it does not, it returns the stanza id to rely on. A concrete example would be to call Room.Join and be able to react to success or - and this is my point - error. Because we could be unable to join a room for several reasons: banned, nick collision, not ready or unexisting room, members-only room. Actually Candy does nothing on these, and do not warn the user. An error callback could help a lot! Same holds for nick change, admin tasks (on that, the code should be rewritten, or should use strophe.muc._modifyPrivilege & related (note the callbacks here again ;) )).

I'm fond of reusing strophe.muc at a small cost (CoffeeScript is not that bad to learn) instead of rewriting thing. I don't know what you think about that now.

And finally, I think locking IM behavior on the assumption that "Candy is a MUC client" is truly sad :( XMPP is an onion, and the core is IM. MUC is "only" an extension. Even if you don't want to bother with user roster and stuff, I would be glad if while refactoring the whole thing, you would keep that in mind and make some room for the do-gooder that would implement it on Candy :) For instance strophe use the concepts of Occupant in room rosters; an Occupant may have a jid property (which is not the "room@domain/nick" occupant jid, but the full jid "user@domain/resource" if provided). Occupants are distinct from users in the user roster, but can be linked to using this jid. This way, it's easy to start a private IM from an Occupant that holds a jid. That also can be really useful for admin tasks (admins can see real JIDs and operate on them). Candy mix the two in ChatUser since it holds bare jid (for me), and occupant jid for others (possibly me, in my case). (cf Core.getUser() problem)

Again, strophe.muc already have this. Probably Candy could be smaller if it relied on it, and you could focus on the real value added of Candy: the user interface!

Thank you for your work and your responsiveness!

My project is to use XMPP as an in-game chat, but letting the choice of using an external client (mobile, desktop) up to the user. I'm planning to sync all instances of an user using Message Carbon and Prosody (because it allows the same user to have multiple resources on a MUC over an unique nick). And to sync MUC presence I will implement some custom presence stanzas.

Sounds interesting!

I want the users to be non-anonymous, so all my rooms are non-anonymous by default. I really want private IM through bare JID instead of through anonymous JID, and I really want to get rid of the issue you mentioned: if you start private chat with the same user from two different rooms, you will have two conversations. That's a tradeoff I cannot live with ;) (So I always use the bare JID sent in presence >x>item[jid] when available).

Ok I understand. However what still is an issue for us is how we want to approach the problem of having the online status of a user without having to send roster accept/denies. A user joining a MUC room usually doesn't want to take care of managing a roster. We certainly could do this in an automatic manner, but this might cause other problems (users not wanting this etc). Maybe I should ask the jdev mailinglist what they think about this issue and how they'd solve it. Maybe the way to go is to support both ways, but I don't currently know if this is managable without a big amount of code.

And last but not least: it also seems to store some rooms etc

That's why I'm asking why you don't rely on it to manage rooms. I didn't dig into this plugin, but since Strophe seems quite bug-free (I corrected one), I imagine that it does this well, I suppose..

Well, the two issues I mentioned are blockers for Candy. Until they are not merged in, I'd have to either use a fork (which is possible and which is what we did until 1.6.0 for strophe itself, but I think should be avoided).

About relying on status codes, for instance I saw at some places that you did Strophe.getResourceFromJid(from) === Candy.Core.getUser().getNick() to check for self-presence stanzas. But you have presence >x>status[code=110] for that.

True. That's something I started working on in the refactoring branch (but it's just on my local machine yet).

Another important flaw is that Candy.Core.getUser().getNick() is unique through the application. But unlike other protocols, XMPP allow to have different nicks on different rooms. Even if you want to keep the same nick over all rooms, you technically can't ensure that if 1) there is a nick policy, 2) there is a nick conflict, 3) this nick is reserved, 4) the user is using an external XMPP client that allow per-room nick change (i.e. near all clients). These edge cases confirm the bad idea behind using this nick instead of code 110 to identify myself.

That's true. As I said, I'm working on status code 110 support and meanwhile working on it, I might be able to change the other part of this behaviour as well.

About that, a nick change is a presence stanza owning the status code 303. And a nick changed by service policy has a status code of 210. (Actually a nick change from A to B is handled as "A leaved the room", "B entered the room").

..Which we handle as of 1.6.0.

I reviewed more carefully the strophe muc plugin. It seems to handle everything provided by MUC, including nick change and non-anonymous rooms, and provide some handlers and callbacks. I admit it is not so well written than I expected, but it's a really good start to me. Even more than a start, it's actually pretty complete IMHO.

I might need to investigate it more then. But as I said: Those two issues are blockers for me.

Which brings me to the callback topic. That's nice to be able to provide a callback when we send a stanza. For instance strophe.muc provide callbacks (success and error) for an info#disco on a room, and when it does not, it returns the stanza id to rely on. A concrete example would be to call Room.Join and be able to react to success or - and this is my point - error. Because we could be unable to join a room for several reasons: banned, nick collision, not ready or unexisting room, members-only room. Actually Candy does nothing on these, and do not warn the user. An error callback could help a lot! Same holds for nick change, admin tasks (on that, the code should be rewritten, or should use strophe.muc._modifyPrivilege & related (note the callbacks here again ;) )).

Hmm, I wouldn't say we do nothing about it, some error cases we handle.. But sure, we don't handle them all and we can improve it. If we would support all (or a lot of the possible) errors, would you like to have an event triggered so you could react on it with your custom logic? My approach with the refactoring is currently that plugins can gain a lot more control of how Candy behaves. This means that every plugin could probably stop propagating an event (similar to how it's done with the fix of #202).

I'm fond of reusing strophe.muc at a small cost (CoffeeScript is not that bad to learn) instead of rewriting thing. I don't know what you think about that now.

Well, it's not that I don't want to learn CS (actually it's pretty easy) just because. It's just my general complaint for library things written in CS which split the ecosystem and hinder contributions.

And finally, I think locking IM behavior on the assumption that "Candy is a MUC client" is truly sad :( XMPP is an onion, and the core is IM. MUC is "only" an extension. Even if you don't want to bother with user roster and stuff, I would be glad if while refactoring the whole thing, you would keep that in mind and make some room for the do-gooder that would implement it on Candy :)

Sure, I'm not against sticking to the standard way. What we're do with Candy is a lightweight MUC client for XMPP. Lightweight is the key: we don't want and can't support every possible use case one could think of. Don't get me wrong: I like XMPP and it's extensibility, what I don't like is pretty much every client out there (because they try to handle everything and usually with a bad user interface). In order to build a lightweight MUC client we have to do some compromises to keep the code manageable.

With the current codebase however, it's hard to improve it in various ways. The optimal thing for me would be to have a common use case (the one which is implemented right now) but if anyone needs to change it in certain ways, he can change the behaviour using the API (read: events) we provide. This, however, is not so easy because of the current codebase.

For instance strophe use the concepts of Occupant in room rosters; an Occupant may have a jid property (which is not the "room@domain/nick" occupant jid, but the full jid "user@domain/resource" if provided). Occupants are distinct from users in the user roster, but can be linked to using this jid. This way, it's easy to start a private IM from an Occupant that holds a jid. That also can be really useful for admin tasks (admins can see real JIDs and operate on them). Candy mix the two in ChatUser since it holds bare jid (for me), and occupant jid for others (possibly me, in my case). (cf Core.getUser() problem)

With "strophe" do you mean the muc plugin?

Again, strophe.muc already have this. Probably Candy could be smaller if it relied on it, and you could focus on the real value added of Candy: the user interface!

Sure: that's why we initially used the muc plugin. But as of now, I had various reasons (the ones I mentioned) to stop using it. I'll think about it again. Maybe I could solve some issues we have with the current codebase by using the muc plugin more.

After talking to the jdev mailinglist (still ongoing, if you want to join ) I think I currently don't want to change our JID behaviour. I'll start now really with the refactoring and meanwhile doing this, I'll have your concerns/opinion in my head. Maybe there's an easy way to help you.

With "strophe" do you mean the muc plugin?

Yes, and also the roster plugin

Interesting conversation! I carefully retained that users feel irritated when their client open a direct IM instead of a MUC chat on an anonymous network. The whole non-anonymous network of Stefan Strigler is what I implemented. His lobby room is also an idea but I don't think it would work (impossible to link nickX in roomX to his nick in the lobby). Here my proposal:

1. store the real_jid of an Occupant if available
2. separate MUC PrivateRoom from IM Chat
3. provide a way to override the behavior when the user click on a nick to start a private chat

About the second point, to take IM apart from MUC PrivateRoom, we should store the isNotConferenceRoomJid in the room data. But this may be a little more complex than that:

Actually, muc plugin takes care to filter MUC-related stanzas. I say that because I had problems with Candy.Core.Event.Jabber.Room.Disco unexpected room creation: I think disco#info IQs result filter is to wide. I had a problem when Strophe reply to presence caps sent by my friends (my roster) with such disco#info iq. Room.Disco catch the response and that leads to wrong room instantiation using a full user jid. The subsequent problem is that point 2) uses room existence to determine if it is a conference room jid or not; and in such case, it determine this is. I replaced the room creation in Candy.Core.Event.Jabber.Room.Disco by a return, and it fixed the problem. I'm not a XMPP wizard, but does that make sense to rely on Disco response to detect a room join instead of a presence stanza? Maybe there is a reason to the presence of this code in Room.Disco.

The key here is probably to store somewhere what we know about an entity bare jid, to easily distinguish user bare and full jids, room jids, and occupant jids (private room jids in Candy). I mentioned this problem on the jdev conference room, and they said:

• unless we are using Message Carbons, we cannot receive a message from an occupand jid if we are not in the concerned room. So looking at known rooms may be sufficient (if this list is kept consistent).
• And if we are using Carbons, a disco iq can reveal the kind of the entity.

The latter should use a code construction based on a callback or a promise (got something from an entity => disco this entity if not known => then proceed accordingly). If Candy could ensure this kind of things (I think it have to, to be interoperable with other clients) that would be marvelous :)

Maybe Strophe and it's plugins (disco plugin?) already handle that, I didn't took time to look for.

Interesting conversation! I carefully retained that users feel irritated when their client open a direct IM instead of a MUC chat on an anonymous network. The whole non-anonymous network of Stefan Strigler is what I implemented. > His lobby room is also an idea but I don't think it would work (impossible to link nickX in roomX to his nick in the lobby). Here my proposal:

1. store the real_jid of an Occupant if available

Yes, that sounds like a good idea.

2. separate MUC PrivateRoom from IM Chat

What do you mean with that exactly?

3. provide a way to override the behavior when the user click on a nick to start a private chat

Jep, that should be fairly easy actually if you have the real jid data.

About the second point, to take IM apart from MUC PrivateRoom, we should store the isNotConferenceRoomJid in the room data. But this may be a little more complex than that:

Maybe there's even a better solution than the isNotConferenceRoomJid variable. It always felt like a hack and not the proper solution. Will take care about this if possible as well.

Actually, muc plugin takes care to filter MUC-related stanzas. I say that because I had problems with Candy.Core.Event.Jabber.Room.Disco unexpected room creation: I think disco#info IQs result filter is to wide. I had a problem when Strophe reply to presence caps sent by my friends (my roster) with such disco#info iq. Room.Disco catch the response and that leads to wrong room instantiation using a full user jid. The subsequent problem is that point 2) uses room existence to determine if it is a conference room jid or not; and in such case, it determine this is. I replaced the room creation in Candy.Core.Event.Jabber.Room.Disco by a return, and it fixed the problem. I'm not a XMPP wizard, but does that make sense to rely on Disco response to detect a room join instead of a presence stanza? Maybe there is a reason to the presence of this code in Room.Disco.

Could you describe this behaviour (maybe in a separate issue) with the debug output? I don't remember the exact reason why we create the room there if it's not yet existing, but I think we had a reason ;) Obviously it should not be the case because the presence stanza is used for that.

The key here is probably to store somewhere what we know about an entity bare jid, to easily distinguish user bare and full jids, room jids, and occupant jids (private room jids in Candy). I mentioned this problem on the jdev conference room, and they said:

• unless we are using Message Carbons, we cannot receive a message from an occupand jid if we are not in the concerned room. So looking at known rooms may be sufficient (if this list is kept consistent).
• And if we are using Carbons, a disco iq can reveal the kind of the entity.

Ok.

The latter should use a code construction based on a callback or a promise (got something from an entity => disco this entity if not known => then proceed accordingly). If Candy could ensure this kind of things (I think it have to, to be interoperable with other clients) that would be marvelous :)

sounds like a good approach.

Maybe Strophe and it's plugins (disco plugin?) already handle that, I didn't took time to look for.

Yeah I need to reread some plugins again about what they do exactly. I'm not up to date there currently. As far as I know, the disco plugin we use does nothing like that, it only replies with caps in case there's a disco info/items request to the user. This approach is IMHO better than handling everything in a strophe plugin.

What do you mean with that exactly?

the isNotConferenceRoomJid variable. It always felt like a hack

The proper solution for these is, to me, to use the disco response that gives the identity of the bare jid: either an user, or a MUC room (or anything else extending the protocol). That will naturally separate MUC PrivateRoom from IM Chat: from=${bare_jid}/resource

• if bare_jid is a conference entity => MUC PrivateRoom
• else if bare_jid is an user => IM Chat

You're right, the disco plugin only provide disco answer capability to the client. It seems tricky - but far from being impossible - to retain stanza processing while the sender is not known (through a disco#info). Reading XEP 30, it seems to be the correct way, requesting the bare JID to know or verify the entity existence and capabilities, in order to display or handle it correctly. I don't know how other XMPP clients handle this case... maybe it would be worthy to ask them for.

Finally, I think it makes sense to instantiate or populate an entity whom we received a disco#info response (as described in #219), and so this issue is directly linked to this refacto.

@pstadler will you commit your connect() code or should I do it? (if you commit, please to the refactoring branch)

If you have free time, then go on. I don't have the changes any more.

FYI: I removed (and probably will remove some more) some tasks from this issue as it's too much currently for a single contributor in one step. We'd need more contributors & a good test suite to do more refactoring.