Closed
Bug 1128060
Opened 9 years ago
Closed 9 years ago
Torn-off tabs cannot be session-restored
Categories
(Firefox :: Session Restore, defect)
Tracking
()
People
(Reporter: erik, Assigned: mossop)
References
Details
Attachments
(1 file)
4.42 KB,
patch
|
billm
:
review+
ttaubert
:
feedback+
|
Details | Diff | Splinter Review |
I encountered this only after switching to e10s, so maybe it's related. 1. Open a page in a new tab. 2. Tear that tab off the window to make it its own window. 3. Close the new window. 4. Hit command-shift-N to restore the window. 5. Leap from your chair in surprise as a window from your deep past springs forth. The torn-off tab is gone forever, making this actually a data-loss bug. In fact, the torn-off tab makes it into neither the Recently Closed Tabs nor the Recently Closed Windows submenus; it's falling through the cracks somewhere. This is in the 2015-01-26 Nightly.
Comment 1•9 years ago
|
||
I saw this on Windows 8.1 as well, using the latest nightly.
tracking-e10s:
--- → ?
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Flags: firefox-backlog+
Updated•9 years ago
|
Assignee: nobody → mconley
Assignee | ||
Updated•9 years ago
|
Points: --- → 3
Flags: qe-verify-
Assignee | ||
Updated•9 years ago
|
Assignee: mconley → dtownsend
Iteration: --- → 39.1 - 9 Mar
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
Switches to setting _permanentKey rather than permanentKey which overrides the property on the as yet unbound XBL. This also allows us to revert the change in bug 1072198 that made permanentKey a read/write property.
Attachment #8575322 -
Flags: review?(wmccloskey)
Updated•9 years ago
|
Iteration: 39.1 - 9 Mar → 39.2 - 23 Mar
Comment 4•9 years ago
|
||
Comment on attachment 8575322 [details] [diff] [review] patch Review of attachment 8575322 [details] [diff] [review]: ----------------------------------------------------------------- Great catch! (Just giving some test feedback.) ::: browser/base/content/test/general/browser_tab_detach_restore.js @@ +1,1 @@ > +add_task(function*() { Nit: "use strict"; @@ +15,5 @@ > + yield new Promise(resolve => whenDelayedStartupFinished(win, resolve)); > + > + is(win.gBrowser.selectedBrowser.permanentKey, key, "Should have properly copied the permanentKey"); > + win.close(); > + yield promiseWaitForFocus(window); yield promiseWindowClosed(win); ? @@ +21,5 @@ > + is(SessionStore.getClosedWindowCount(), 1, "Should have restore data for the closed window"); > + > + win = SessionStore.undoCloseWindow(0); > + yield BrowserTestUtils.waitForEvent(win, "load"); > + yield BrowserTestUtils.browserLoaded(win.gBrowser.tabs[0].linkedBrowser); I think we should rather wait for the initial tab's SSTabRestored event. @@ +27,5 @@ > + is(win.gBrowser.tabs.length, 1, "Should have restored one tab"); > + is(win.gBrowser.selectedBrowser.currentURI.spec, uri, "Should have restored the right page"); > + > + win.close(); > + yield promiseWaitForFocus(window); yield promiseWindowClosed(win);
Attachment #8575322 -
Flags: feedback+
Comment on attachment 8575322 [details] [diff] [review] patch Review of attachment 8575322 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/general/browser_tab_detach_restore.js @@ +7,5 @@ > + > + let tab = gBrowser.addTab(); > + let loadPromise = BrowserTestUtils.browserLoaded(tab.linkedBrowser); > + tab.linkedBrowser.loadURI(uri); > + yield loadPromise; We really need to find the right pattern for tests to add a tab with a given URL and wait for it to load. The problem with this version, I think, is that if about:blank loads really quickly then you'll get a load event for that instead of for the intended URI. I'm not sure how likely this is, but it seems possible according to my understanding. I saw a similar problem in bug 1132566 too. Maybe you could talk to Steven MacLeod and figure out something to add to BrowserTestUtils?
Attachment #8575322 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5197df93e0b4
Backed out for bc1 orange: https://treeherder.mozilla.org/logviewer.html#?job_id=2238309&repo=fx-team https://hg.mozilla.org/integration/fx-team/rev/2b519f79e37a
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 8•9 years ago
|
||
Looks like this wasn't at fault: https://hg.mozilla.org/integration/fx-team/rev/5a79c295029a
Flags: needinfo?(dtownsend)
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5a79c295029a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
You need to log in
before you can comment on or make changes to this bug.
Description
•