philc / vimium

The hacker's browser.
https://chrome.google.com/webstore/detail/vimium/dbepggeogbaibhgnhhndojpepiihcmeb
MIT License
23.11k stars 2.48k forks source link

'visitPreviousTab' should go to the next most recent visited tab *that is open* #2567

Open 9aaron opened 7 years ago

9aaron commented 7 years ago

I have often closed a tab and intuitively wanted to go to the next most recent visited tab that is open by pressing the 'visitPreviousTab' key.

However in this case Vimium seems to only remember the previously visited tab as the one that was closed and does nothing.

So if a tab is closed, 'visitPreviousTab' should go to the next most recent visited tab that is open.

innaterebel commented 7 years ago

In the meantime waiting for developers to respond, you can use T then Enter for a workaround. (The list listed by T is in most recent order)

smblott-github commented 7 years ago

I cannot reproduce this.

(We're talking about the visitPreviousTab command, ^, right?)

If I delete tabs then I end up visiting the most recently visited tab which has not bee deleted. (Which is correct.)

innaterebel commented 7 years ago

For me it is:

  1. Open three tabs (A, B, C).
  2. Ctrl+Tab to tab A, then tab B, then tab C. (i.e. hit Tab three times while holding Ctrl)
  3. Close tab B.

Now ^ does nothing, I have to 2^ to get to tab A.

smblott-github commented 7 years ago

Thanks, @innaterebel, but I still cannot reproduce this.

@mrmr1993 Can you?

mrmr1993 commented 7 years ago

I can't reproduce this either...

I can get weird behaviours if I change tabs over a certain rate (>=2 tabs/sec, due to #1233). Does the issue still present if you change tabs more slowly?

In the meantime waiting for developers to respond, you can use T then Enter for a workaround.

If this works but ^ doesn't, something really strange is going on: they both use the same data source (BgUtils.tabRecency) and do much the same thing with it.

innaterebel commented 7 years ago

Okay then... got it! It is the chrome://flags/#enable-fast-unload flag causing this issue (for me at least). Probably not worth fixing in that case.

However this flag has existed for a couple years, and has not been shown progress outside its initial implementation, so it is surprising it now actually changes things. And interesting as you mentioned why ^ stops working while T is unaffected.

Forgot to mention that I am on Dev channel of Chrome so the flag may not do the same thing for current stable versions.

mrmr1993 commented 7 years ago

And interesting as you mentioned why ^ stops working while T is unaffected.

I overlooked an important detail: ^ uses the tab list in TabRecency, whereas T gets its own tab list from chrome.tabs.query. You can build master with 812e3f2 to make ^ behave the same as T.

It is the chrome://flags/#enable-fast-unload flag causing this issue

Repro'd. We don't get tabs.onRemoved events when tabs (without beforeunload/unload handlers?) are closed. You might want to open a Chromium issue to this effect.

innaterebel commented 7 years ago

Will do and report back if it gets addressed (or not). Thank you for investigation!

innaterebel commented 6 years ago

As this is bumped, I am conveniently reporting that the Chromium-side issue I opened got completely ignored 😁

Nevertheless, for the flag has been existing for ages, and the fact that I found enabling it on Chrome for mobile induces crashes, the flag is most likely hasn't been maintained for a very long time, and likely will not be. So I guess it is better just forget it and leave it disabled. (although it does make tab closing on desktop a bit faster)