Closed Bug 1087966 Opened 10 years ago Closed 9 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 39
Iteration:
39.1 - 9 Mar
Tracking Status
e10s m6+ ---
firefox39 --- verified

People

(Reporter: jimm, Assigned: mossop)

References

Details

(Keywords: dogfood)

Attachments

(2 files, 1 obsolete file)

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.
Summary: Tab drag to new window results in incorrect "New Tab" tab title → e10s - Tab drag to new window results in incorrect "New Tab" tab title
Story Addition: Tab title is incorrect "New Tab" and urlbar is blank
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.
Keywords: dogfood
Summary: e10s - Tab drag to new window results in incorrect "New Tab" tab title → e10s - Tab drag to new window results in incorrect "New Tab" tab title and a largely broken tab
In addition, returning it to the first window makes the tab completely blank.
See Also: → 1105542
Blocks: e10s
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.
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
Flags: qe-verify+
Flags: firefox-backlog+
OS: Windows 7 → All
Hardware: x86_64 → All
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.
Blocks: 1127904
+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.
Blocks: 1134375
Blocks: 1088126
Assignee: nobody → dtownsend
Status: NEW → RESOLVED
Iteration: --- → 38.3 - 23 Feb
Points: --- → 2
Closed: 9 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Attached patch patchSplinter Review
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.
Attachment #8566536 - Flags: feedback?(davidp99)
Blocks: 1094328
(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.
Attachment #8566536 - Flags: feedback?(davidp99)
Attached file MozReview Request: bz://1087966/Mossop (obsolete) —
/r/4051 - Bug 1087966: Fix tab detach in e10s windows.

Pull down this commit:

hg pull review -r 424742b3fbba8ea2725680d78c07c7536d96093f
Attachment #8566786 - Flags: review?(mconley)
(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 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.
Attachment #8566786 - Flags: review?(mconley) → review+
(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
Iteration: 38.3 - 23 Feb → 39.1 - 9 Mar
https://hg.mozilla.org/mozilla-central/rev/c9da89e679b2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
QA Contact: cornel.ionce
Depends on: 1136938
Depends on: 1136948
Depends on: 1136953
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.
Status: RESOLVED → VERIFIED
Depends on: 1136910
Attachment #8566786 - Attachment is obsolete: true
Attachment #8618437 - Flags: review+
You need to log in before you can comment on or make changes to this bug.