Closed Bug 904521 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(firefox23 unaffected, firefox24+ verified, firefox25+ verified, firefox26+ verified)

VERIFIED FIXED
Firefox 26
Tracking Status
firefox23 --- unaffected
firefox24 + verified
firefox25 + verified
firefox26 + verified

People

(Reporter: standard8, Assigned: florian)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
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)
Attached patch Patch (obsolete) — Splinter Review
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.
Assignee: nobody → florian
Attachment #790212 - Flags: review?(mixedpuppy)
Attached patch Patch (obsolete) — Splinter Review
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
Attachment #790212 - Attachment is obsolete: true
Attachment #790212 - Flags: review?(mixedpuppy)
Attachment #790392 - Flags: review?(mhammond)
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
Attachment #790462 - Flags: review?(mhammond)
regarding my above comments referencing bug 840807, I actually meant bug 891221.
Attachment #790392 - Flags: review?(mhammond) → review+
Attachment #790462 - Flags: review?(mhammond) → review+
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
Attachment #790392 - Attachment is obsolete: true
Attachment #791002 - Flags: review+
Attachment #791002 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/73fff1e427d3
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
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 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
Attachment #791002 - Flags: approval-mozilla-beta?
Attachment #791002 - Flags: approval-mozilla-beta?
Attachment #791002 - Flags: approval-mozilla-beta+
Attachment #791002 - Flags: approval-mozilla-aurora?
Attachment #791002 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Verified as fixed on Firefox 24 beta 5 - 20130822154523 - on Windows 7, Mac OS X 10.7.5 and Ubuntu 13.04.
QA Contact: ioana.budnar
Also verified on Firefox 25:
Mac OS X 10.8.4 - 20130912004004
Ubuntu 13.04 - 20130911004006
Windows 7 - 20130912004004
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).
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.