445461 - MRU tab order lost when restoring a session

Status: VERIFIED FIXED

(Firefox :: Keyboard Navigation, defect)

Description

[Dão Gottwald [:dao]]

The most-recently-used tab order from bug 395980 is currently lost when a session is restored. We should probably store that information.

Comment 1

[Veritas]

please make sure that there is an option to use most-recently-used tab or the default (system wide) logical ordering.  if you have ten to twenty tabs up, i challenge that absolutely no one can remember without a lot of difficulty how many times they would have to hit ctrl tab to get to the window.  mru is not logical behavior anyway.

Comment 2

[Veritas]

if a tab is three to the left, you ought to be able to hit ctrl-shift-tab,tab,tab to reach it.  at current, you have no way of knowing how many times you have to hit it, and waste your time looking at the preview panel

Comment 3

[u88484]

Veritas:  once again, read the dependencies to bug 395980 and stop spamming random CTRL+TAB related bugs!  Here is the link, https://bugzilla.mozilla.org/showdependencytree.cgi?id=395980&hide_resolved=1 most importantly; bug 445499:  "Allow Ctrl+Tab preview to display in tab bar order"

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8371885 [details] [diff] [review]
patch

Review of attachment 8371885 [details] [diff] [review]:
-----------------------------------------------------------------

When restoring windows other than the first one (e.g. restoring multiple windows, undo closing a window) we're listening for the load event and will call ss.restoreWindow() on the next tick, which might be before delayedStartup() is called. ctrlTab would thus initialize after SSWindowStateReady has fired. I think ctrlTab should do the same on .init() as on SSWindowStateReady and initialize with a sorted list of visible tabs.

Comment 6

[Dão Gottwald [:dao]]

It also occurs to me that hidden tabs need to be included, otherwise the MRU data will be missing when switching tab groups.

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8372164 [details] [diff] [review]
patch v2

Review of attachment 8372164 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch wrt to hidden tabs.

Comment 9

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/f34185ced01a

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/f34185ced01a

Comment 11

[ithinc]

Currently, the lastAccessed stamp is not set on a never accessed new background tab, which will have lastAccessed = 0 stored. This does not match how ctrlTab handles a new tab. 
I'll open a new bug for this.

Comment 12

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 30.0b5
Build Id:20140515140857
OS: Windows 7 x64, Mac Os X 10.8.5, Ubuntu 13.04 x64