Closed
Bug 1000198
Opened 10 years ago
Closed 10 years ago
fix sessionstore tests that remove the original tab
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 32
People
(Reporter: froydnj, Assigned: ttaubert)
Details
Attachments
(1 file)
4.72 KB,
patch
|
smacleod
:
review+
|
Details | Diff | Splinter Review |
There are just a handful of sessionstore tests that destroy the original tab: TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_522545.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-apptabs.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-apptabs_ondemand.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-reload.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-select.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_600545.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_601955.js | Test destroyed original tab I suppose some of these extend to tabview tests, too: TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug595601.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug600812.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug608153.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug608158.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug608405.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug624847.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug626455.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug633788.js | Test destroyed original tab TEST-UNEXPECTED-FAIL | browser/components/tabview/test/browser_tabview_bug685692.js | Test destroyed original tab What I don't understand is why these particular tests fail. Other tests save, modify, and restore the window state and don't seem to have any problems with retaining the original tab. But these tests also save, modify, and restore the window state, yet fail. (I'm using the patch in bug 805068 to determine whether the original tab was destroyed; that patch may have bugs...) Tim, do you have any intuition as to what might be going wrong here?
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 2•10 years ago
|
||
We treat the selected tab differently than other tabs when restoring a session into a window. To minimize flickering we make sure to not remove the selected tab and thus we should make sure this is always the first one.
Updated•10 years ago
|
Attachment #8412733 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 3•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/644472efabae
Whiteboard: [fixed-in-fx-team]
Comment 4•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/644472efabae
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Comment 5•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/dbe507c02444 https://hg.mozilla.org/releases/mozilla-beta/rev/e078f53fd234
Comment 6•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/e078f53fd234
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
Comment 7•10 years ago
|
||
Given bug 805068, should this be backed out or are these changes somehow useful by themselves?
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 8•10 years ago
|
||
The changes aren't particularly useful but they also really don't hurt. I think it's easier to just let them in the tree.
Flags: needinfo?(ttaubert)
Comment 9•10 years ago
|
||
Comment on attachment 8412733 [details] [diff] [review] 0001-Bug-1000198-Fix-sessionstore-tests-that-remove-the-o.patch There is however at least this misleading comment, as there's no point in ensuring this: >--- a/browser/components/sessionstore/test/head.js >+++ b/browser/components/sessionstore/test/head.js >+ // Ensure setBrowserState() doesn't remove the initial tab. >+ gBrowser.selectedTab = gBrowser.tabs[0];
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9) > >+ // Ensure setBrowserState() doesn't remove the initial tab. > >+ gBrowser.selectedTab = gBrowser.tabs[0]; That bit might indeed be a bit misleading only because we don't really mention why we want to ensure the initial tab isn't removed. We can remove the comment or the line but I guess I don't really feel strongly.
You need to log in
before you can comment on or make changes to this bug.
Description
•