Closed Bug 804591 Opened 7 years ago Closed 7 years ago

message window remains minimized when I click on a name


(Firefox Graveyard :: SocialAPI, defect)

Not set


(firefox18+ verified, firefox19 verified)

Firefox 19
Tracking Status
firefox18 + verified
firefox19 --- verified


(Reporter: mccr8, Assigned: markh)



(1 file, 2 obsolete files)

If I don't have a chat window open for somebody, then when I click on the message dropdown for that person, their chat window appears, but if it was minimized when I last closed it, it appears minimized.

With the chat app on Facebook's actual webpage, if you do that, the chat window will not appear minimized, which I think is better, because you obviously want to interact with it if you just caused it to be created.

In my particular case, I was on a page with a gray background, so I didn't even notice at first that the tiny minimized bar appeared.
Does this only happen when the sidebar is hidden? I vaguely recall differences in behavior between sidebar- vs. worker-opened chat windows, but I forget whether that's a limitation we enforce.
I can't reproduce this.  If the sidebar is hidden, the chat window will indeed open minimized - but whether it is initially opened normally or minimized, clicking the titlebar of the chat reliably toggles the minimized state for me.

Andrew - can you still reproduce this?
Flags: needinfo?(continuation)
Yes, clicking on the title bar of the chat itself toggles the minimized state, my comment was about how it initially appears after you click on a particular person in the chat menu thing that drops down from the icon in the upper right. I posit that it should appear non-minimized any time you click on a conversation.

Sorry, I missed your comment before, Gavin.

sidebar opened, chat already open, minimized: chat does not unminimize
sidebar opened, chat not already open: chat appeared unminimized as expected
sidebar opened, chat already open, not minimized: stays unminized as expected
sidebar closed, chat already open minimized, or not open: chat appears minimized
Flags: needinfo?(continuation)
When you say "I posit that it should appear non-minimized any time you click on a conversation.", do you mean "...  click on a *contact in the sidebar*" or "... click on a *chat window*"?

I think the former - in which case I'm inclined to agree and isn't what actually happens.  If the latter, then I can't repro and/or still don't understand :)
The latter works fine for me.

I mean click on a contact/conversation in the "Messages" menu that drops down when you click on the little blue messages icon in the upper right. Sorry, I guess I am using the wrong jargon here.
This should fix most cases of this (although it will depend slightly on the provider behaviour)
Assignee: nobody → mhammond
Attachment #676837 - Flags: review?(mixedpuppy)
Comment on attachment 676837 [details] [diff] [review]
If a request is made to open a minimized chat, restore the chat.

Review of attachment 676837 [details] [diff] [review]:

r+ with that minor part of the test changed.

::: browser/base/content/test/browser_social_chatwindow.js
@@ +69,5 @@
> +          port.postMessage({topic: "test-chatbox-open"});
> +          break;
> +        case "chatbox-opened":
> +          is(, "ok", "the sidebar says it got a chatbox");
> +          if (seen_opened) {

Handle the first case first, so make this:
if (!seen_opened) {
} else {
Attachment #676837 - Flags: review?(mixedpuppy) → review+
Sadly bug 798083 not only caused a conflict with this patch, it also introduced a regression.  Specifically, if a worker tried to open the same chat twice (and the worker always gets a minimized chat), the chatbox would have magically become unminimized - it should have stayed minimized in that case.

This patch is very similar to the old one but also fixes that regression and adds a new test specifically for that case.

Even though the patch is similar, it isn't similar enough that I feel comfortable carrying the r= forward - sorry jaws :)
Attachment #676837 - Attachment is obsolete: true
Attachment #679009 - Flags: review?(jaws)
Comment on attachment 679009 [details] [diff] [review]
updated to fix (and test) regression from bug 798083

Review of attachment 679009 [details] [diff] [review]:

::: browser/base/content/socialchat.xml
@@ +364,5 @@
>            let cb = this.chatboxForURL.get(aURL);
>            if (cb) {
>              cb = cb.get();
>              if (cb.parentNode) {
> +              this.showChat(cb, aMode != "minimized");

I would rather that this variable get passed in as a string to showChat, and then put the comparison within showChat so it is easier to follow the code path here.

This would be particularly helpful in the oncommand attribute of the nubMenu since the extra boolean 'true' doesn't really describe to the reader what impact that is likely to have.
Attachment #679009 - Flags: review?(jaws) → review-
Comment on attachment 679009 [details] [diff] [review]
updated to fix (and test) regression from bug 798083

I'll grant review now with the expectation that you'll make that change to the API before landing.
Attachment #679009 - Flags: review- → review+
Attached patch As landedSplinter Review
Attachment #679009 - Attachment is obsolete: true
Attachment #681344 - Flags: review+
Backed out in for the weirdest set of later tests timing out.
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Is this Social API fix still desired on FF18? If so, please nominate asap.
Comment on attachment 681344 [details] [diff] [review]
As landed

This one is user-facing and still applies, so nominating.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: The social chat-boxes will not be in a consistent or user-expected state.
Testing completed (on m-c, etc.): Landed on 19 some time ago, includes tests
Risk to taking this patch (and alternatives if risky): Low risk, limited to social chat
String or UUID changes made by this patch: None
Attachment #681344 - Flags: approval-mozilla-beta?
Attachment #681344 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.
Yeah, this has been working for me recently. There's some slight weirdness with getting a scroll bar in the text input box, but that's a different issue. I'll file a new bug if I figure out how to reproduce it or whatever.
Oh, I was thinking of another bug. I'll try this out at some point and file a new bug if I am still seeing issues of some sort...
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.