Closed
Bug 954083
Opened 11 years ago
Closed 11 years ago
Chat Rooms should be available to JavaScript protocols
Categories
(Chat Core :: General, defect)
Chat Core
General
Tracking
(Not tracked)
RESOLVED
FIXED
0.3a1
People
(Reporter: clokep, Assigned: clokep)
References
Details
Attachments
(1 file, 1 obsolete file)
3.98 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 648 at 2011-01-07 02:19:00 UTC *** This bug will handle making purpleIChatRoomField and purpleIChatRoomFieldValue available to JavaScript protocols. See http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/public/purpleIAccount.idl#66 for the interface.
Assignee | ||
Comment 1•11 years ago
|
||
*** Original post on bio 648 as attmnt 475 at 2011-01-14 04:59:00 UTC *** Implements ChatRoomField and ChatRoomFieldValues and sets some samples for jsTestProtocol. This might not apply properly with some of the other changes to jsProtoHelper.jsm.
Attachment #8352218 -
Flags: review?(florian)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment 2•11 years ago
|
||
Comment on attachment 8352218 [details] [diff] [review] v1.0 *** Original change on bio 648 attmnt 475 at 2011-01-14 15:33:13 UTC *** >diff --git a/purple/purplexpcom/src/jsProtoHelper.jsm b/purple/purplexpcom/src/jsProtoHelper.jsm >+ let type = typeof chatRoomField.default; >+ if (type == "number") That's confusing. I would prefer: let type; if ((typeof chatRoomField.default) == "number") (not sure if the parenthesis around the typeof operator are needed by the way) >+ chatRoomFields.push(new ChatRoomField( >+ chatRoomField.label, chatRoomFieldName, type, chatRoomField.required, >+ chatRoomField.min, chatRoomField.max >+ )); The comment I made in the bug about UserSplits could apply here too. the ChatRoomField constructor could take only 2 parameters: the identifier and the values. But we did it this way for options, so... anyway... >+ getChatRoomDefaultFieldValues: function(aDefaultChatName) { >+ if (!this.chatRoomFields) >+ return EmptyEnumerator; >+ >+ let chatRoomDefaultFieldValues = []; >+ for (let chatRoomFieldName in this.chatRoomFields) { >+ let chatRoomField = this.chatRoomFields[chatRoomFieldName]; >+ chatRoomDefaultFieldValues[chatRoomFieldName] = chatRoomField.default; >+ } >+ return new ChatRoomFieldValues(chatRoomDefaultFieldValues); I think this code would be more readable if you removed the "chatRoom" prefix from the local variables. >+ChatRoomFieldValues.prototype = { >+ getValue: function(aIdentifier) { >+ if (this.values.hasOwnProperty(aIdentifier)) >+ return this.values[aIdentifier]; The indent isn't right here. You have two unneeded spaces. >+ return null; I think this could fit in 80 columns by the way: this.values.hasOwnProperty(aIdentifier) ? this.values[aIdentifier] : null >diff --git a/purple/purplexpcom/src/jsTestProtocol.js b/purple/purplexpcom/src/jsTestProtocol.js >+ chatRoomFields: { >+ "channel": {"label": "_Channel Field", "required": true}, >+ "channelDefault": {"label": "_Channel Field (with default)", >+ "default": "Default Value"}, >+ "password": {"label": "_Password Field", "default": "", "isPassword": true, >+ "required": false}, >+ "sampleIntField": {"label": "_Int Field", "default": 4, "required": true, >+ "min": 0, "max": 10} I think the " could be removed from the keys here, as we did in the UsernameSplit bug.
Attachment #8352218 -
Flags: review?(florian) → review-
Assignee | ||
Comment 3•11 years ago
|
||
*** Original post on bio 648 as attmnt 480 at 2011-01-14 15:58:00 UTC *** Fixes the comments and simplifies the code in a few other instances.
Attachment #8352223 -
Flags: review?(florian)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8352218 [details] [diff] [review] v1.0 *** Original change on bio 648 attmnt 475 at 2011-01-14 15:58:57 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352218 -
Attachment is obsolete: true
Comment 5•11 years ago
|
||
Comment on attachment 8352223 [details] [diff] [review] v2.0 *** Original change on bio 648 attmnt 480 at 2011-01-14 17:05:41 UTC *** Looks good, with a few nits that I'll fix before pushing (missing semicolon, trailing comma, this.min/max lines that could be moved inside the INT specific block, canJoinChat: true needed to make the example work).
Attachment #8352223 -
Flags: review?(florian) → review+
Assignee | ||
Comment 6•11 years ago
|
||
*** Original post on bio 648 at 2011-01-14 18:09:18 UTC *** Checked in http://hg.instantbird.org/instantbird/rev/61fc80a569d3
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Target Milestone: --- → 0.3a1
You need to log in
before you can comment on or make changes to this bug.
Description
•