1292049 - Ctrl+Tab gets stuck after reopening a closed tab

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Dão Gottwald [:dao]]

STR:

- enable "Ctrl+Tab cycles through tabs in recently used order"
- have a bunch of tabs open
- close a tab
- reopen the tab
- hit Ctrl+Tab

This is because SessionStore.jsm may set lastAccessed on a selected tab, and the lastAccessed getter(!) doesn't account for the tab being selected as of bug 980231.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

Comment 2

[Dão Gottwald [:dao]]

[Tracking Requested - why for this release]: we're now exposing this feature to end users (bug 1116787)

Comment 3

[Mike de Boer [:mikedeboer]]

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

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

Well-spotted.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/510f0b4792de
Prevent tab.lastAccessed from being set to a discrete value when the tab is selected. r=mdeboer

Comment 5

[Dão Gottwald [:dao]]

Backed out because of this:

04:27:00 INFO - 54 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_354894_perwindowpb.js | Uncaught exception - at chrome://browser/content/tabbrowser.xml:2023 - TypeError: t.updateLastAccessed is not a function

updateLastAccessed should definitely exist there. Looks like a case of XBL being broken.

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch v2

Moved the updateLastAccessed call to the tab's XBL constructor. Shouldn't be necessary but it does appear to fix the browser_354894_perwindowpb.js issue.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/343370281fd7
Prevent tab.lastAccessed from being set to a discrete value when the tab is selected. r=mdeboer

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/343370281fd7

Comment 9

[Dão Gottwald [:dao]]

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

Approval Request Comment
[Feature/regressing bug #]: bug 1116787
[User impact if declined]: see bug summary and comment 0
[Describe test coverage new/current, TreeHerder]: has a test
[Risks and why]: fairly straightforward and well-understood fix, low risk
[String/UUID change made/needed]: none

Comment 10

[Nawaf Chowdhury Akash]

I have reproduced this bug with nightly 51.0a1 (2016-08-04) on windows 10, 32 bit.

The bug’s fix is now verified on Latest Nightly 51.0a1.

Build ID:20160805030444.
User Agent:Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101 Firefox/51.0.

Comment 11

[Dão Gottwald [:dao]]

(In reply to Akash from comment #10)
> I have reproduced this bug with nightly 51.0a1 (2016-08-04) on windows 10,
> 32 bit.
> 
> The bug’s fix is now verified on Latest Nightly 51.0a1.
> 
> Build ID:20160805030444.
> User Agent:Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101
> Firefox/51.0.

Thanks!

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked since it's a recent regression.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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

Regression fixed, Aurora50+

Comment 14

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/43e9e32c32a8

Comment 15

[Cornel Ionce [:noni] [Hubs QA]]

Confirming that this issue no longer reproduces on Win 10x64, Ubuntu 12.04 x86 and Mac OS X 10.11.6 using latest 50.0a2 Aurora, build ID 20160913004005.