Closed
Bug 954418
Opened 10 years ago
Closed 10 years ago
Joining chat should unhide conversation if already existing
Categories
(Instantbird Graveyard :: Contacts window, defect)
Instantbird Graveyard
Contacts window
Tracking
(Not tracked)
RESOLVED
FIXED
1.1
People
(Reporter: benediktp, Assigned: florian)
References
Details
(Whiteboard: [1.1-wanted])
Attachments
(1 file)
5.78 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 984 at 2011-08-24 12:14:00 UTC *** Hidden conversations are currently not unhidden when the user typed the same name into the "Join Chat" dialog (for whatever reason). I think using the dialog is enough 'wish to use this conversation' to unhide it if it already exists.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [1.1-wanted]
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 984 as attmnt 814 at 2011-09-13 11:10:00 UTC *** As much as I dislike it (especially the fact that it has prpl-specific behaviors), this patch seems to work. My previous attempts involved making purpleIAccount::joinChat return the conversation if it already existed; given how libpurple create chat rooms, it turned out to be almost impossible. I then tried to add a purpleIAccount::FindChat method, but it didn't work any better. There's no way in libpurple to search for an existing chat based on the chat components (the info we put in purpleIChatRoomFieldValues), and there's no API to generate the chat room name based on the components either, so adding one in the C++ code would have been as hacky as what my patch does, except writing it would have been more painful and we would have had to write something too in jsProtoHelper... If anybody has a better idea for a simpler/less hackish fix, that would be welcome. Review comments on the attached patch are welcome too of course :).
Attachment #8352557 -
Flags: review?
Comment 2•10 years ago
|
||
*** Original post on bio 984 at 2011-09-13 12:08:51 UTC *** (In reply to comment #1) > Created an attachment (id=814) [details] > Patch > > As much as I dislike it (especially the fact that it has prpl-specific > behaviors), this patch seems to work. So by my understanding this will only refocus IRC, GTalk or Jabber chats, correct? For Jabber overrides (i.e. GTalk), could we QI to purpleIOverrideProtocol and use the basePrpl.id to grab them all (or maybe this isn't worth the change). r+ if we don't think this is worth it / this isn't possible.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment 3•10 years ago
|
||
Comment on attachment 8352557 [details] [diff] [review] Patch *** Original change on bio 984 attmnt 814 at 2011-09-13 15:08:32 UTC *** So purpleIOverrideProtocol is actually unused, so we'll ignore that idea for now.
Attachment #8352557 -
Flags: review? → review+
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 984 at 2011-09-13 17:36:03 UTC *** (In reply to comment #2) > (In reply to comment #1) > > As much as I dislike it (especially the fact that it has prpl-specific > > behaviors), this patch seems to work. > > So by my understanding this will only refocus IRC, GTalk or Jabber chats, > correct? Yes! Thanks for the quick review. :) Fixed: https://hg.instantbird.org/instantbird/rev/e323e76ce887
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.1
You need to log in
before you can comment on or make changes to this bug.
Description
•