NiklasGollenstede / unload-tabs

WebExtension that unloads tabs and prevents tabs from loading
https://addons.mozilla.org/addon/unload-tabs/
Mozilla Public License 2.0
69 stars 13 forks source link

When closing a tab, the wrong tab gets focused next #6

Closed chris-morgan closed 7 years ago

chris-morgan commented 7 years ago

I do not expect this to change Firefox’s behaviour. If the next tab that would be loaded is pending, so be it. I don’t want this extension to interfere.

Still, the behaviour I’m observing is I think not even the behaviour you intended.

I have lots of tabs. Here’s an extract:

When tab 6 is focused and I close it with Ctrl+W, I expect tab 7 to be focused, as it is in stock Firefox.

Instead, tab 4 is focused (!). Or in other similar circumstances, tab 5 (the previous tab).

I just want normal Firefox behaviour. I just want the TabClose event listener to be removed. Can changing the behaviour please be shifted to an option (and probably fixed so it matches what Firefox does more closely)?

I believe this extension should be focused only on unloading tabs.

chris-morgan commented 7 years ago

I see that the onClose preference was added recently but isn’t in the AMO build.

I request then the addition of another option, “just leave it alone, let Firefox do its thing”. This should be the default, in my opinion.

NiklasGollenstede commented 7 years ago

I see that the onClose preference was added recently but isn’t in the AMO build.

Yes, I did that in response to am AMO review. I uploaded a current build on the beta channel (at the bottom of the page. You are very welcome to download and test that, if nothing is broken, I'll move it to release soon.

I request then the addition of another option, “just leave it alone, let Firefox do its thing”.

Yup, will do that.

This should be the default, in my opinion.

I am not going to change the behaviour for existing installations; at least not with announcing it, and I don't intend to implement an update notification. On the other hand I could change the behaviour for new installations by setting the pref on installation. I think 1 or 3 would be most suitable.

Instead, tab 4 is focused (!). Or in other similar circumstances, tab 5 (the previous tab).

I could not reproduce that tab 4 get's focused. At default setting of onClose, tab 5 should be and is getting focused.

chris-morgan commented 7 years ago

Yeah, tab 4 getting focused was inconsistent and doesn’t make a great deal of sense given the code. I don’t know how it was that it happened, but it definitely happened at least twice (the first time being tab 7 → 4 in that sample).

I think the Firefox default is the sensible thing. I believe it’s more complex than just “the opener, to the right”, because if I open a new tab now with Ctrl+Shift+A or Alt+Enter from the address bar and then close that tab I expect to get back to this tab, even though this tab isn’t marked as the opener of that tab. With this extension, though, I just wind up at the last tab rather than this tab.

Setting it to the Firefox default on installation seems good.

NiklasGollenstede commented 7 years ago

I believe it’s more complex than just “the opener, to the right”

It really isn't: there is a owner property on the xul elements that handles exactly this (and which isn't the same as window.opener.

It would be nice if you could download the latest beta release from AMO and test it. The "that opened the tab, to the right (Firefox default)" option should do what you want. I only added the ignore option for completeness sake. Should it not work as expected please feel free to reopen this issue!

chris-morgan commented 7 years ago

My first tests seem to be saying that 3 is now behaving as I expect.

Thanks!