Closed
Bug 1003096
Opened 10 years ago
Closed 10 years ago
Remove tab reordering feature implemented in bug 480148
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 32
Tracking | Status | |
---|---|---|
firefox32 | --- | fixed |
People
(Reporter: ttaubert, Assigned: ttaubert)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
12.07 KB,
patch
|
smacleod
:
review+
|
Details | Diff | Splinter Review |
3.04 KB,
patch
|
smacleod
:
review+
|
Details | Diff | Splinter Review |
browser_480148.js fails permanently with the patch from bug 715376 applied as with per-tab event queues it doesn't make sense anymore to reorder tabs before starting to restore them i.e. send async messages to them. To unblock bug 715376 we should remove _setTabsRestoringOrder() and the test. With restore_on_demand=true being the default nowadays it doesn't seem relevant anymore anyway.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8414410 -
Flags: review?(smacleod)
Comment 2•10 years ago
|
||
Comment on attachment 8414410 [details] [diff] [review] 0001-Bug-1003096-Remove-tab-reordering-feature-implemente.patch Review of attachment 8414410 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the doc changes. ::: browser/components/sessionstore/src/SessionStore.jsm @@ +2444,5 @@ > this._setWindowStateReady(aWindow); > return; > } > > + // Select the selected tab. I think something like "If provided, set the selected tab" might be clearer? Also, the documentation for the "aSelectTab" argument is pretty inadequate. Can we take this opportunity to call out the fact that it's a 1-based index, and that a value of 0 means don't select a tab? Maybe something like: * @param aSelectTab * Index of the tab to select. This is a 1-based index where "1" indicates * the first tab should be selected, and 0 indicates that the currently * selected tab will not be changed.
Attachment #8414410 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Thanks for the suggestions, I will steal and copy them.
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/434ffe0d33b2
Assignee | ||
Comment 5•10 years ago
|
||
Backed out for tabview failures: https://hg.mozilla.org/integration/fx-team/rev/83c3a7471051
Assignee | ||
Comment 6•10 years ago
|
||
This patch fixes browser_tabview_bug595601.js so that it actually waits until the test session has been restored before continuing. I think this should also fix bug 964009.
Attachment #8415173 -
Flags: review?(smacleod)
Assignee | ||
Comment 7•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=54bd6b24f821
Comment 8•10 years ago
|
||
Comment on attachment 8415173 [details] [diff] [review] 0002-Bug-1003096-Make-browser_tabview_bug595601.js-wait-u.patch Review of attachment 8415173 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me ::: browser/components/tabview/test/browser_tabview_bug595601.js @@ +47,5 @@ > } > > function testRestoreWithHiddenTabs() { > + TabsProgressListener.setCallback(function (needsRestore, isRestoring) { > + if (4 < needsRestore) Since we're moving it, might as well put braces around?
Attachment #8415173 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f55a3989aee0 https://hg.mozilla.org/integration/fx-team/rev/e057952237dc
Whiteboard: [fixed-in-fx-team]
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f55a3989aee0 https://hg.mozilla.org/mozilla-central/rev/e057952237dc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Comment 11•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fde20263e8c9 https://hg.mozilla.org/releases/mozilla-beta/rev/66518d59cdbd *JUST* the bug 964009 test fix :)
Comment 12•10 years ago
|
||
Sorry, got a little status flag happy there.
status-firefox30:
fixed → ---
status-firefox31:
fixed → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•