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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benediktp, Assigned: florian)

References

Details

(Whiteboard: [1.1-wanted])

Attachments

(1 file)

*** 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.
Blocks: 954412
Whiteboard: [1.1-wanted]
Attached patch PatchSplinter Review
*** 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?
*** 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 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+
*** 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.