Closed
Bug 779976
Opened 12 years ago
Closed 12 years ago
Set priority with XMPP
Categories
(Thunderbird :: Instant Messaging, enhancement)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 18.0
People
(Reporter: cyril.glapa, Assigned: cyril.glapa)
References
()
Details
Attachments
(1 file, 4 obsolete files)
3.27 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0 Build ID: 20120724191344 Steps to reproduce: I added a XMPP account. Actual results: I cannot set a priority for my resource. Expected results: I should be able to set a priority for my resource.
Updated•12 years ago
|
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #657323 -
Flags: review?(florian)
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #657323 -
Attachment is obsolete: true
Attachment #657323 -
Flags: review?(florian)
Attachment #657329 -
Flags: review?(florian)
Comment 3•12 years ago
|
||
Comment on attachment 657329 [details] [diff] [review] Patch v2 >diff -r e11462508947 chat/protocols/facebook/facebook.js >--- a/chat/protocols/facebook/facebook.js Mon Aug 27 21:21:49 2012 -0400 >+++ b/chat/protocols/facebook/facebook.js Fri Aug 31 18:46:38 2012 +0200 >@@ -19,6 +19,7 @@ > FacebookAccount.prototype = { > __proto__: XMPPAccountPrototype, > get canJoinChat() false, >+ get priority() 0, Can't this just inherit from the super class? This would allow someone to go into about:config and set the priority then. (Same for GTalk.) >diff -r e11462508947 chat/protocols/xmpp/xmpp.js >--- a/chat/protocols/xmpp/xmpp.js Mon Aug 27 21:21:49 2012 -0400 >+++ b/chat/protocols/xmpp/xmpp.js Fri Aug 31 18:46:38 2012 +0200 >@@ -29,6 +29,7 @@ > options: { > resource: {get label() _("options.resource"), > get default() XMPPDefaultResource}, >+ priority: {get label() _("options.priority"), default: 0}, Was there thought to putting this here or was it just randomly chosen? (I'm assuming it was chosen to group it with "resource", which kind of seem related...) Do we definitely not want this option for GTalk and Facebook exposed in the UI? (I think we don't for Facebook, but I could potentially be convinced for GTalk...)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #657329 -
Attachment is obsolete: true
Attachment #657329 -
Flags: review?(florian)
Attachment #657348 -
Flags: review?(florian)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #657348 -
Attachment is obsolete: true
Attachment #657348 -
Flags: review?(florian)
Attachment #657356 -
Flags: review?(florian)
Comment 6•12 years ago
|
||
Comment on attachment 657356 [details] [diff] [review] Patch v4 Thanks Cyril! This looks good to me, but as we were pair programming on it, Patrick may want to double check :).
Attachment #657356 -
Flags: review?(florian)
Attachment #657356 -
Flags: review?(clokep)
Attachment #657356 -
Flags: review+
Updated•12 years ago
|
Attachment #657356 -
Flags: review?(clokep) → review+
Comment 7•12 years ago
|
||
Comment on attachment 657356 [details] [diff] [review] Patch v4 >diff -r e11462508947 chat/protocols/xmpp/xmpp.jsm >--- a/chat/protocols/xmpp/xmpp.jsm Mon Aug 27 21:21:49 2012 -0400 >+++ b/chat/protocols/xmpp/xmpp.jsm Fri Aug 31 19:44:17 2012 +0200 >@@ -1207,6 +1207,9 @@ > this.reportDisconnected(); > }, > >+ get priority() >+ this.prefs.prefHasUserValue("priority") ? this.getInt("priority") : 0, >+ > /* Set the user status on the server */ > _sendPresence: function() { > delete this._shouldSendPresenceForIdlenessChange; >@@ -1232,6 +1235,14 @@ > let time = Math.floor(Date.now() / 1000) - this._idleSince; > children.push(Stanza.node("query", Stanza.NS.last, {seconds: time})); > } >+ let priority = this.priority; Now that we don't override it anymore from facebook.js and gtalk.js, I wonder if we should keep the priority getter, or just inline it.
Updated•12 years ago
|
Comment 8•12 years ago
|
||
I don't see any test with this :(
Comment 9•12 years ago
|
||
- inlined the priority getter - replaced the large if/else if block with Math.max(-128, Math.min(127, this.getInt("priority"))) that we find more readable (this was discussed on IRC).
Attachment #657356 -
Attachment is obsolete: true
Comment 10•12 years ago
|
||
Comment on attachment 658157 [details] [diff] [review] Patch v5 Review over IRC.
Attachment #658157 -
Flags: review+
Comment 11•12 years ago
|
||
Landed for IB as http://hg.instantbird.org/instantbird/rev/d83296ace3cd - Thanks!
Updated•12 years ago
|
Assignee: nobody → cyril.glapa
Status: NEW → ASSIGNED
Comment 12•12 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8100c66adb6a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
You need to log in
before you can comment on or make changes to this bug.
Description
•