Closed
Bug 1010342
Opened 11 years ago
Closed 9 years ago
Join chat dialog should prepopulate the server field for XMPP accounts
Categories
(Chat Core :: XMPP, defect, P3)
Chat Core
XMPP
Tracking
(Not tracked)
RESOLVED
FIXED
1.6
People
(Reporter: h0ckeysk8er, Assigned: abdelrahman)
References
Details
Attachments
(1 file, 2 obsolete files)
4.86 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release) Build ID: 20140506152807 Steps to reproduce: Clicking on the Join Chat button opens the "Join chat" dialog for the XMPP account. Actual results: The dialog opens with the account name and nickname fields prepopulated. For my XMPP server, multi-user chat is located at rooms.server.domain.com instead of server.domain.com. You must manually input the server name and if it differs from your account server, you get no warning or indication that it is incorrect when the dialog is submitted to join the chat. Expected results: The server field of the dialog should be prepopulated with the server name for the multi-user chat service similar to the behavior of other IM clients such as Pidgin. TB should perform a service discovery query to find the MUC server per http://xmpp.org/extensions/xep-0045.html#disco-service There should also be some error or warning when the join chat dialog is submitted and it is an invalid request to the server. Additionally, it would be ideal if there would also be a separate button to click to list the available rooms/channels on that server per http://xmpp.org/extensions/xep-0045.html#disco-rooms with the ability to select a room/channel to join.
Reporter | ||
Updated•11 years ago
|
Priority: -- → P3
Updated•11 years ago
|
Blocks: 954959
Component: Instant Messaging → XMPP
OS: Windows 7 → All
Product: Thunderbird → Chat Core
Hardware: x86_64 → All
Version: 24 → trunk
Comment 1•11 years ago
|
||
Thanks for your report. Bugs / enhancement requests are usually more actionable if you request only one thing per bug. In your description I can see at least 3 things: - do service discovery to know the name of the conference server of your XMPP server and have a sane default value - display somewhere a list of the channels available. - giving the user clear feedback when joining a MUC failed for some reason.
Comment 3•11 years ago
|
||
(In reply to h0ckeysk8er from comment #2) > I can split it into 3 if you like. Let's keep this one as the service discovery, please file the other two as separate issues. Thanks!
Assignee | ||
Comment 4•9 years ago
|
||
Confirming, this is a part of Service Discovery (XEP-0030) [1] [1] http://xmpp.org/extensions/xep-0030.html#info-basic
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 5•9 years ago
|
||
Comment 6•9 years ago
|
||
Comment on attachment 8625402 [details] [diff] [review] rev 1 - service discovery for a muc service Review of attachment 8625402 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for tackling this! ::: chat/protocols/xmpp/xmpp.jsm @@ +1136,4 @@ > > /* Called when the XMPP session is started */ > onConnection: function() { > this.reportConnecting(_("connection.downloadingRoster")); Add a comment above this: // Request the roster. The account will be marked as connected when this is complete. @@ +1138,5 @@ > onConnection: function() { > this.reportConnecting(_("connection.downloadingRoster")); > let s = Stanza.iq("get", null, null, Stanza.node("query", Stanza.NS.roster)); > > /* Set the call back onRoster */ Remove this comment and join the line below with the previous block. @@ +1145,5 @@ > + // XEP-0030 and XEP-0045 (6): Service Discovery. > + // Queries Server for Associated Services. > + let iq = Stanza.iq("get", null, this._jid.domain, > + Stanza.node("query", Stanza.NS.disco_items)); > + this.sendStanza(iq, aStanza => { This callback is quite long and will probably get longer as more features are added later, so please move it to its own method (e.g. onServiceDiscovery).
Attachment #8625402 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8625402 -
Attachment is obsolete: true
Attachment #8625502 -
Flags: review?(aleth)
Comment 8•9 years ago
|
||
Comment on attachment 8625502 [details] [diff] [review] rev 2 - service discovery for a muc service Review of attachment 8625502 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/xmpp/xmpp.jsm @@ +1135,5 @@ > /* XMPPSession events */ > > + // Called when the XMPP session is started. > + // Request the roster. The account will be marked as connected when this is > + // complete. Move the "request the roster" comment inside (just above this.reportConnecting) as it describes what the first block of code does. @@ +1148,5 @@ > + Stanza.node("query", Stanza.NS.disco_items)); > + this.sendStanza(iq, aStanza => { > + this.onServiceDiscovery(aStanza); > + return true; > + }); this.sendStanza(iq, this.onServiceDiscovery, this); @@ +1211,5 @@ > this.WARN("Unhandled presence stanza."); > }, > > + // XEP-0030: Discovering services and their features that are supported by > + // the server. Trailing whitespace.
Attachment #8625502 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8625502 -
Attachment is obsolete: true
Attachment #8625529 -
Flags: review?(aleth)
Comment 10•9 years ago
|
||
Comment on attachment 8625529 [details] [diff] [review] rev 3 - service discovery for a muc service Review of attachment 8625529 [details] [diff] [review]: ----------------------------------------------------------------- Can you change the join command accordingly? Also, when I tested this patch with "Join chat" from the menu, it didn't work: Timestamp: 23/06/2015 23:55:44 Error: uncaught exception: Some required fields are empty! Source File: chrome://instantbird/content/menus.js Line: 140
Attachment #8625529 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to aleth [:aleth] from comment #10) > Can you change the join command accordingly? There is a separate bug for that (bug 1172360).
Comment 12•9 years ago
|
||
Comment on attachment 8625529 [details] [diff] [review] rev 3 - service discovery for a muc service Review of attachment 8625529 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for adding this!
Attachment #8625529 -
Flags: review- → review+
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in
before you can comment on or make changes to this bug.
Description
•