After landing of bug 1010342, we can obtain the domain if it is not passed to _parseJID in xmpp.jsm (e.g command join is used "/join room").

Comment on attachment 8626300 [details] [diff] [review] rev 1 - handle join without server domain Review of attachment 8626300 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/xmpp/xmpp.jsm @@ +1456,5 @@ > resource: match[3] > }; > + let jid; > + if (!result.node && result.domain) > + [result.node, result.domain] = [result.domain, result.node]; Changing parseJid in this way for the convenience of /join is not a good idea. JIDs without nodes do happen (e.g. when sending/receiving service discovery messages) and we want to be sure to handle them correctly. JIDs without domains are (if I remember correctly) not allowed. Instead, move this swap to the /join code.

(In reply to aleth [:aleth] from comment #2) > Changing parseJid in this way for the convenience of /join is not a good > idea. JIDs without nodes do happen (e.g. when sending/receiving service > discovery messages) and we want to be sure to handle them correctly. JIDs > without domains are (if I remember correctly) not allowed. > > Instead, move this swap to the /join code. This change is not only for join, but also for any case like (node/resource or node). Because the regular expression parses node in domain and that is not correct as we do not provide "@" to be parsed as domain.

(In reply to Abdelrhman Ahmed [:abdelrhman] from comment #3) > (In reply to aleth [:aleth] from comment #2) > > Changing parseJid in this way for the convenience of /join is not a good > > idea. JIDs without nodes do happen (e.g. when sending/receiving service > > discovery messages) and we want to be sure to handle them correctly. JIDs > > without domains are (if I remember correctly) not allowed. > > > > Instead, move this swap to the /join code. > > This change is not only for join, but also for any case like (node/resource > or node). > Because the regular expression parses node in domain and that is not correct > as we do not provide "@" to be parsed as domain. Aha! Well spotted. I didn't realize parseJid was broken. Maybe the regex could be improved? jid = [ localpart "@" ] domainpart [ "/" resourcepart ] In any case, and as this is an important function, I think this is a perfect case for test-driven development. Please add an xpschell test for parseJid. You can write the test first and then change the code until your tests pass ;) Documentation: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests The full spec is rfc 6122 btw.

(In reply to aleth [:aleth] from comment #4) > > Because the regular expression parses node in domain and that is not correct > > as we do not provide "@" to be parsed as domain. Hmm, I'm not sure I understood you correctly here. Just to be clear, the @ is optional, the domainpart is not: jid = [ localpart "@" ] domainpart [ "/" resourcepart ]

(In reply to aleth [:aleth] from comment #5) > Hmm, I'm not sure I understood you correctly here. Just to be clear, the @ > is optional, the domainpart is not: > jid = [ localpart "@" ] domainpart [ "/" resourcepart ] Ah, so the regex works well for _parseJID, we need to add another parser beside join command to check if the domain part is provided and handle that otherwise. jid = localpart [ "@" domainpart ] [ "/" resourcepart ]

Comment on attachment 8626461 [details] [diff] [review] rev 2 - handle join without server domain Review of attachment 8626461 [details] [diff] [review]: ----------------------------------------------------------------- Don't forget to change the help string too! It's great you've added a test despite not changing _parseJid! Please move the test to a separate bug though, as it's not related to this bug. ::: chat/protocols/xmpp/xmpp-commands.jsm @@ +17,5 @@ > > // Get account object. > function getAccount(aConv) getConv(aConv)._account; > > +function parseJID(aJID, aMucService) { You don't need all this duplication if instead you change parseDefaultChatName on the account.

Comment on attachment 8626732 [details] [diff] [review] rev 3 - handle join without server domain Review of attachment 8626732 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but please fix the help string!

Comment on attachment 8626765 [details] [diff] [review] rev 4 - handle join without server domain Review of attachment 8626765 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!