1250847 - Opening a room from the panel with a single tab open, closes the window in e10s mode

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Mark Banner (:standard8)]

STR:

1) Turn on e10s mode in FF
2) Set the loop.remote.autostart pref to true
3) Restart FF
4) Ensure you only have one tab open
5) Open the Loop panel
6) Click a button, e.g. "new room" or "get started"

Actual Results

The window closes

Expected Results

Window stays open and performs the correct action.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: prefix DOMWindowClose and DOMTitleChanged messages with the Social: prefix

Mark, this is what we've been through discussing this afternoon in code.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8723210 [details] [diff] [review]
Patch v1: prefix DOMWindowClose and DOMTitleChanged messages with the Social: prefix

Review of attachment 8723210 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=Standard8 I am tempted to say we should support both old & new APIs for a bit, but I think we should uplift the core patch to aurora as well to be safe there.

Comment 3

[Mike de Boer [:mikedeboer]]

(In reply to Mark Banner (:standard8) from comment #2)
> Looks good. r=Standard8 I am tempted to say we should support both old & new
> APIs for a bit, but I think we should uplift the core patch to aurora as
> well to be safe there.

I think we should uplift it aurora indeed. I'll create a PR with the loop changes, so you can merge it at your leasure ;-)

Comment 4

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/c75bc39acae8f6dc6f66cf3e479fc2ddd301c08a
Bug 1250847: prefix DOMWindowClose and DOMTitleChanged messages with the Social: prefix to assure they won't arrive at the global messageManager instance that TabBrowser is listening to. It mixes things up in twisted, unexpected ways if we don't. r=Standard8

Comment 5

[GitHub Autolander Bot]

Attachment: [loop] mikedeboer:bug-1250847-prefixMessages > mozilla:master

Comment 6

[Mike de Boer [:mikedeboer]]

Comment on attachment 8723815 [details] [review]
[loop] mikedeboer:bug-1250847-prefixMessages > mozilla:master

Carrying over r=Standard8.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c75bc39acae8

Comment 8

[Mark Banner (:standard8)]

Now that the fix has made its way into nightly, I've merged it into the tree as well:

https://github.com/mozilla/loop/commit/863a2bf381940b9bcc170677faccb2ad05d4c15e
https://github.com/mozilla/loop/commit/990e1bda97e9403f40b780e0e9ae99c7f841f5a3

Comment 9

[Mike de Boer [:mikedeboer]]

Comment on attachment 8723210 [details] [diff] [review]
Patch v1: prefix DOMWindowClose and DOMTitleChanged messages with the Social: prefix

Approval Request Comment
[Feature/regressing bug #]: e10s
[User impact if declined]: once we enable e10s support for Hello, opening a conversation for the first time in a window with one tab open, the window will be closed.
[Describe test coverage new/current, TreeHerder]: landed on m-c, tests pass.
[Risks and why]: minor - Hello system add-on need to be updated to work.
[String/UUID change made/needed]: n/a.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Regression if we have e10s enabled, tracking and marking affected.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8723210 [details] [diff] [review]
Patch v1: prefix DOMWindowClose and DOMTitleChanged messages with the Social: prefix

Fix for an e10s regression that affects Hello, please uplift to aurora.

Comment 13

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d8a8e7757a1a