Last Comment Bug 793036 - Close all service windows on logout.
: Close all service windows on logout.
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: unspecified
: All All
-- normal (vote)
: Firefox 18
Assigned To: Mark Hammond [:markh]
: Shane Caraveo (:mixedpuppy)
Depends on: 793935
  Show dependency treegraph
Reported: 2012-09-20 16:49 PDT by Mark Hammond [:markh]
Modified: 2012-10-17 16:46 PDT (History)
3 users (show)
markh: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Close all chat windows on logout (2.02 KB, patch)
2012-09-20 23:41 PDT, Mark Hammond [:markh]
jaws: review+ approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Mark Hammond [:markh] 2012-09-20 16:49:04 PDT
All service windows (eg, chat windows) must be closed when the user logs out of the service (ie, when the provider tells is there is a 'null' profile)
Comment 1 User image Mark Hammond [:markh] 2012-09-20 23:41:02 PDT
Created attachment 663310 [details] [diff] [review]
Close all chat windows on logout

Also added a utility function haveLoggedInUser() which a couple other things reuse.
Comment 2 User image Jared Wein [:jaws] (please needinfo? me) 2012-09-21 20:03:20 PDT
Comment on attachment 663310 [details] [diff] [review]
Close all chat windows on logout

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

::: browser/base/content/browser-social.js
@@ +173,5 @@
>    // Whether the chats can be shown for this window.
>    get canShow() {
> +    if (!SocialUI.haveLoggedInUser()) {
> +      return false;
> +    }

No need for the curly brackets if it is a one-line if block.
Comment 4 User image Ryan VanderMeulen [:RyanVM] 2012-09-24 17:58:48 PDT

Should this have a test?
Comment 5 User image Mark Hammond [:markh] 2012-09-25 16:33:42 PDT
tests via bug 793935
Comment 6 User image :Gavin Sharp [email:] 2012-10-02 18:19:48 PDT
Comment 7 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-10-17 16:46:58 PDT
No QA verification since this has in-testsuite coverage. Please add verifyme keyword to request manual verification.

Note You need to log in before you can comment on or make changes to this bug.