1087966 - e10s - Tab drag to new window results in incorrect "New Tab" tab title and a largely broken tab

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Jim Mathies [:jimm]]

str:

1) open one e10s window
2) drag a tab out to create a new window for the tab

result: the tab drags out, window is created, but the new tab has the incorrect tab title.

Comment 1

[martin]

Story Addition: Tab title is incorrect "New Tab" and urlbar is blank

Comment 2

[Mark Banner (:standard8)]

Testing with 2014-11-10 nightly build, I'm also seeing the following items broken:

- refresh typically doesn't work
- Right-click menu (lists everything)
- Dragging to a retina display doesn't re-display the tab, causing half-size content.

Nothing is shown on the browser console when the tab is loaded.

Clicking a link is enough to get it working to some extent, although the right-click menu is still broken.

Comment 3

[pedrogfrancisco]

In addition, returning it to the first window makes the tab completely blank.

Comment 13

[Ray Satiro]

I also have this problem when I use 'Move to New Window'. From an e10s tab I get a new e10s window and the tab is named 'New tab' and although there is no url address the content is moved there. From a non-e10s tab I get a new e10s window and the tab is named 'New tab' but the content is not moved. Also the non-e10s tab then can't be closed.

Comment 14

[Andreas Schoeneck]

If a new window is opened directly (via menu),
 the behavior is correct at first:

- right click menu works as expected
- the reload function works
- the tab's title is correct

If the tab is then re-integrated into the formerly existing window, the observed behavior like described above is back:

- title is "New Tab"
- address lost in address bar
- page does not initially show up, instead the loading animation is shown (changing to another tab and back is required)
- reload is broken (even no hover indication)

However, 
- context menu seems to only show relevant items
- console is not empty

Comment 15

[Jim Michaels]

even without e10s, same behavior.
visit answers.microsoft.com, do edits.
drag tab off window (oops! user error!)
notice that tab says "New Tab" but page still has original content. 
drag tab back onto window.
refresh of any kind (buttons, F5, rt-click-refresh button) and address bar are disabled. address bar for that tab is also blank! back button is disabled and fwd button is gone.
rt-click-view page info does nothing.
if I ctrl-b to bookmark it, the URL is about:blank
that precious page is totally lost.

workaround: in this particular forum, if I use the navigation to go to a different category or some other link, the back button becomes enabled and the address bar is back to normal and so is the tab, but the history is lost.

Comment 21

[u524575]

+1 to this, it creates a copy of the window that is unusable, the original tab is 'stuck' and cannot be deleted from the original window.  I'm pretty sure I'm not even using e10s, but it does happen with that as well.

Comment 22

[Dave Townsend [:mossop]]

Attachment: patch

Increasing the list of fields we swap for remote browsers fixes many of the issues this bug describes. Also updating the browser for some of those fields that require it helps.

David, I noticed that I had to have many of those components deregister from the old message manager and register with the new one. Does the message manager not swap? I suspect there re many other message registrations that will have to be updated if this is the case.

Comment 23

[David Parks [:handyman]]

(In reply to Dave Townsend [:mossop] from comment #22)
> Does the message manager not swap?

It does:
https://mxr.mozilla.org/mozilla-central/source/dom/base/nsFrameLoader.cpp#1038

This mirrors the non-e10s swap behavior:
https://mxr.mozilla.org/mozilla-central/source/dom/base/nsFrameLoader.cpp#1321

The additional fields that you needed to swap don't surprise me at all but I guess I'd expect the message manager listeners to behave the same as non-e10s.  I could be missing something tho.

Comment 24

[Dave Townsend [:mossop]]

Attachment: MozReview Request: bz://1087966/Mossop

/r/4051 - Bug 1087966: Fix tab detach in e10s windows.

Pull down this commit:

hg pull review -r 424742b3fbba8ea2725680d78c07c7536d96093f

Comment 25

[Dave Townsend [:mossop]]

(In reply to David Parks [:handyman] from comment #23)
> (In reply to Dave Townsend [:mossop] from comment #22)
> > Does the message manager not swap?
> 
> It does:
> https://mxr.mozilla.org/mozilla-central/source/dom/base/nsFrameLoader.
> cpp#1038
> 
> This mirrors the non-e10s swap behavior:
> https://mxr.mozilla.org/mozilla-central/source/dom/base/nsFrameLoader.
> cpp#1321
> 
> The additional fields that you needed to swap don't surprise me at all but I
> guess I'd expect the message manager listeners to behave the same as
> non-e10s.  I could be missing something tho.

Nope I was a little confused. We swap the browser's frameloader and then we swap the frameloader's message manager. So a browser element has the same message manager before and after the swap. This means that any objects that are listening to the message manager and retaining some kind of state about the remote side have to be copied to the new browser and switch to listening to the new browser's message manager.

Comment 26

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8566786 [details]
MozReview Request: bz://1087966/Mossop

https://reviewboard.mozilla.org/r/4049/#review3333

This looks fine to me, and works very well (although when I drag a tab into a window without focus, I get a perma-spinner on it until I switch the tabs on it... separate bug?)

Just one small question and one small nit - but nothing I'm sweating about.

::: toolkit/modules/RemoteFinder.jsm
(Diff revision 1)
> +    this._messageManager.addMessageListener("Finder:CurrentSelectionResult",this);

Nit - space before this

::: browser/base/content/test/general/browser_tab_dragdrop.js
(Diff revision 1)
>      EventUtils.synthesizeMouseAtCenter(doc.body, {}, win);

Do we still need this click then?

::: browser/base/content/test/general/browser_tab_dragdrop.js
(Diff revision 1)
> +      let target = content.document.body;
> +      let rect = target.getBoundingClientRect();
> +      let left = (rect.left + rect.right) / 2;
> +      let top = (rect.top + rect.bottom) / 2;
> +
> +      let utils = content.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
> +                         .getInterface(Components.interfaces.nsIDOMWindowUtils);
> +      utils.sendMouseEvent("mousedown", left, top, 0, 1, 0, false, 0, 0);
> +      utils.sendMouseEvent("mouseup", left, top, 0, 1, 0, false, 0, 0);

This stuff is so good.

Comment 27

[Dave Townsend [:mossop]]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #26)
> Comment on attachment 8566786 [details]
> MozReview Request: bz://1087966/Mossop
> 
> https://reviewboard.mozilla.org/r/4049/#review3333
> 
> This looks fine to me, and works very well (although when I drag a tab into
> a window without focus, I get a perma-spinner on it until I switch the tabs
> on it... separate bug?)

That is bug 1087969

Comment 28

[Dave Townsend [:mossop]]

https://hg.mozilla.org/integration/fx-team/rev/c9da89e679b2

Comment 29

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

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

Comment 31

[Cornel Ionce [:noni] [Hubs QA]]

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

Confirming the fix on latest Nightly, build ID: 20150305072141.
Closing this as the remaining issues found while testing are already tracked in bug 1087969 and 1094328.

Comment 33

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1087966: Fix tab detach in e10s windows.