904521 - Detaching the chat window and then opening another window, e.g. persona log in, closes the chat window

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Banner (:standard8)]

STR:

1) Log into Talkilla
2) Open a conversation (aka chat) window
3) Click the little arrow to expand the chat window into a separate window
4) Do some browsing, visit a site that requires persona log in
5) Click the button to open up the persona log in window

Expected Results

The conversation window stays open. The persona window is opened.

Actual Results

The conversation window is closed. The persona window is opened.

Comment 1

[Shane Caraveo (:mixedpuppy)]

simplified STR:

* use demo provider to open chat window
* use arrow button to tear off chat window
* open any new window (e.g. switch back to browser window, open new browser window)

Comment 2

[Florian Quèze [:florian]]

Attachment: Patch

When opening a new browser window, SocialUI's initialization dispatches a provider-set notification (http://hg.mozilla.org/mozilla-central/annotate/3c61cc01a3b1/browser/base/content/browser-social.js#l51) that causes a call to SocialChatBar.closeWindows (http://hg.mozilla.org/mozilla-central/annotate/3c61cc01a3b1/browser/base/content/browser-social.js#l84) which closes all detached chat windows.
I think we want this code executed only when disabling a provider, so I moved it to Social.jsm.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: Patch

updated patch with some changes I wanted.  switching review to markh due to below reasons.

Really, this should be fixed on top of bug 840870, however this fix needs uplift to aurora, and I don't want to uplift bug 840870.  We should get this in and modify after uplift OR have this be a branch patch for aurora.

https://tbpl.mozilla.org/?tree=Try&rev=b94d9e586637

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: fix on top of bug 840870

this is a better fix given changes in bug 840870.  The previous patch would be better for uplift.

https://tbpl.mozilla.org/?tree=Try&rev=4af652404700

Comment 5

[Shane Caraveo (:mixedpuppy)]

regarding my above comments referencing bug 840807, I actually meant bug 891221.

Comment 6

[Shane Caraveo (:mixedpuppy)]

attachment #790462 [details] [diff] [review] https://hg.mozilla.org/integration/fx-team/rev/73fff1e427d3

Comment 7

[Shane Caraveo (:mixedpuppy)]

Attachment: aurora branch patch

carrying forward r+, this patch is updated to apply on aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new functionality allowing chat windows to be detached from browser window
User impact if declined: independent chat windows are closed when new windows are opened
Testing completed (on m-c, etc.): m-c branch patch is slightly different, though removing the same issue.  this patch was tested via try on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/73fff1e427d3

Comment 9

[Alex Keybl [:akeybl]]

Don't we want this all the way up to Beta? Approving for Aurora 25 in the least, since this is low risk and seems desirable.

Comment 10

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 791002 [details] [diff] [review]
aurora branch patch

hmm, yes we do need this in 24.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new functionality allowing chat windows to be detached from browser window
User impact if declined: independent chat windows are closed when new windows are opened
Testing completed (on m-c, etc.): m-c branch patch is slightly different, though removing the same issue.  this patch was tested via try on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/18fab23c0fed
https://hg.mozilla.org/releases/mozilla-beta/rev/1f447b598a31

Comment 12

[Ioana (away)]

Verified as fixed on Firefox 24 beta 5 - 20130822154523 - on Windows 7, Mac OS X 10.7.5 and Ubuntu 13.04.

Comment 13

[Ioana (away)]

Also verified on Firefox 25:
Mac OS X 10.8.4 - 20130912004004
Ubuntu 13.04 - 20130911004006
Windows 7 - 20130912004004

Comment 14

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 5.1; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0

Verified as fixed on latest Aurora 26.0a2 (buildID: 20131003004003).