Closed Bug 1000198 Opened 6 years ago Closed 6 years ago

fix sessionstore tests that remove the original tab

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 32
Tracking Status
firefox30 --- fixed
firefox31 --- fixed
firefox32 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: froydnj, Assigned: ttaubert)

Details

Attachments

(1 file)

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)
Filed bug 1001521 for tabview tests.
Flags: needinfo?(ttaubert)
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.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8412733 - Flags: review?(smacleod)
Attachment #8412733 - Flags: review?(smacleod) → review+
https://hg.mozilla.org/mozilla-central/rev/644472efabae
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Given bug 805068, should this be backed out or are these changes somehow useful by themselves?
Flags: needinfo?(ttaubert)
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 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];
(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.
No longer blocks: 805068
You need to log in before you can comment on or make changes to this bug.