M-Reimer / undoclosetab

Undo Close Tab Add-on for Firefox
GNU General Public License v3.0
92 stars 16 forks source link

Addon re-opens random tabs periodically #19

Closed Kinematics closed 4 years ago

Kinematics commented 7 years ago

Periodically, attempting to undo a closed tab will undo the wrong closed tab. I have had instances of it re-opening a tab that I know I last closed several days ago, with hundreds of tabs opened and closed since then. I've had it re-open an entire window when I only asked it to undo a tab closing. It sometimes re-opens a tab in a different window, despite the option to only show tabs closed in the current window being set, which means I shouldn't even be able to reference the tab to re-open it. Most frequently, it will just re-open a random tab out of the last few dozen that I've closed.

I cannot reproduce this on demand. It just happens sporadically.

Currently using latest Nightly of Firefox (v58, Oct 13). This behavior has been going on ever since I started using this addon (v57, when Undo Closed Tabs Button addon stopped working).

M-Reimer commented 7 years ago

I've filed a bug for this: https://bugzilla.mozilla.org/show_bug.cgi?id=1408575 I remember that I had this in the past, too. And probably this exact same effect happens with all WebExtension based Addons for tab restore.

Do you remember if you had this problem with just clicking the button (restore last tab) or does it only happen when picking something from the context menu?

If it also happens when just clicking, I think there is not much I can do to fix this. In this case, I just ask the API for the last closed tab in the current window. If this doesn't work reliably, then the API is broken "somewhere".

Would actually help to be able to reproduce this, so if someone has this problem, please tell a bit about when it happend. The more details we get, the bigger is the chance that at some time someone will find a way to reproduce the problem.

M-Reimer commented 7 years ago

May it be possible, that you somethings switch Firefox versions with the same profile? Maybe sometimes "stable release" and sometimes "nightlies". I remember to have done that when I first had this problem.

M-Reimer commented 7 years ago

I didn't have this for quite some time now. If someone has this Issue again, please comment here, so we maybe find a way to track down the reason for this problem.

M-Reimer commented 6 years ago

There is some chance that this problem is fixed in 4.1.1.

There was a problem that, when focusing windows, the "onWindowFocus" event got fired several times. This resulted in the context menu update routine to run several times in parallel which caused menu item creation to crash (resulted in "Id already exists" errors in browser console).

Kinematics commented 6 years ago

Oh, wow, sorry. Lost track of this bug quite a while ago.

I'm currently updated to version 4.1.1. It still occurs as recently as in the last week. I rarely use the context menu, so almost all instances have been from just clicking the toolbar button.

This particular profile does not get used in alternate versions of Firefox. It remains locked to the Nightly channel, because 57+ broke everything. I don't want to risk incompatible changes across versions.

GTHub23 commented 6 years ago

Not sure if it will help, but found a discussion on Reddit that has a temporary fix for this bug: https://www.reddit.com/r/firefox/comments/7pmsz3/clear_recently_closed_tab_window_list/

Essentially, using about:config to change the settings for "browser.sessionstore.max_tabs_undo" & "browser.sessionstore.max_windows_undo" from their defaults to "0", wait around 15 secs, then change them back, which empties out the recently closed tabs/windows file. Personally, I restart Firefox then reset the values back to their defaults, then repeat if this issue arise again. It's probably only a temporary solution hopefully iy helps in the long run

tbertels commented 5 years ago

Not sure if it will help, but found a discussion on Reddit that has a temporary fix for this bug: https://www.reddit.com/r/firefox/comments/7pmsz3/clear_recently_closed_tab_window_list/

Unfortunately, it's not a workaround for this bug, it's a workaround for the fact that there's no "Clear Recently Closed Tab/ Window List" in the History menu.

KrasnayaPloshchad commented 5 years ago

Is it possible to ignore such pages via the history API?

M-Reimer commented 5 years ago

There is a good chance that this is also a corrupted session. Try deleting the files/folders mentioned here: https://github.com/M-Reimer/undoclosetab/issues/64#issuecomment-525032919

M-Reimer commented 5 years ago

There may be an even easier way to workaround this but I didn't test. Maybe someone can try this. Be sure that it's OK if all your open tabs are lost! Bookmark the ones you really need!

Should also result in a fresh session to start with. Recheck "Restore previous session" if you like.

M-Reimer commented 4 years ago

Seems like this bug here has better description and way to reproduce this problem:

https://bugzilla.mozilla.org/show_bug.cgi?id=1538119

I'll have a closer look at this now. If I can reproduce the problem, then there may be a chance to workaround this bug.

M-Reimer commented 4 years ago

The issue behind this bug is that the so-called "session ID's", which should identify the closed tabs and windows, can duplicate as soon as you restore a previous session. When restoring a session it seems like old session ID's are restored back into the list of closed tabs/windows but when closing more tabs and windows the newly added session ID's are counted from zero again without taking care for existing session ID's. If one session ID exists twice, then it is random which one of the two will be restored.

There are two ways to workaround this problem.

  1. The easiest way would be to auto-reset the "recently closed list" whenever the browser restarts. This would be the "cleanest" way as clearing the whole recently closed list would give us unique session ID's in this session and we can profit from a "real" tab restore including all metadata. But this would mean that even with "session restore" activated, your browser will always start with an empty "recently closed tabs" list in both, the context menu of my Add-on and the browser history.
  2. The second way would be to filter the session ID entries and only show the ID with the newest modification date. This way only the newest are shown in the context menu. But for all duplicated entries it would be impossible to restore them using the proper backend (as this opens a random one). The only way to "restore" them is opening a new tab with the URL of the closed tab. All meta data would be lost like back/forward history, scroll position, stuff entered into forms, ...

As many users would prefer to have a proper restore with all possible data that was in this tab, I would prefer to go for solution "1". I hope there are not too many users who liked to have recently closed tabs saved between browser sessions...

tbertels commented 4 years ago

There may be a third solution: auto-reset the "recently closed list" whenever the browser restarts like in 1. but recreate them in the list of Undo Close Tab. These would just reopen a new tab with the URL like in 2. , but it's better than nothing. New closed tabs would of course have full functionality like in 1.

M-Reimer commented 4 years ago

Great idea! Thanks for sharing! I think that's what I'll try to do, but this will take a few days for implementing and testing. If done well I could share some code between Desktop and Android now as on Android I already restore based on opening tabs with the last URL (as Android has no "sessions" API at all).

M-Reimer commented 4 years ago

First implementation of a possible fix: https://github.com/M-Reimer/undoclosetab/commit/22386317f65df9b48534a584094691737b2c99af

This still needs more testing on both, Desktop Firefox and Android but the basic idea should work. On startup all contents of the "sessions" storage are moved into an internal list. Then all stored recently closed tabs and windows are removed. The used list of recently closed tabs from this point on is a combination of my internal list and the "sessions" storage.

Kinematics commented 4 years ago

When you're ready for beta testing, let me know how to put together something to test, and I'll help out.

M-Reimer commented 4 years ago

Please try this: https://github.com/M-Reimer/undoclosetab/releases/tag/6.0.0b1 But you'll have to use "Firefox developer version" as I'm not able to sign this version in a way that allows installing into the "regular" Firefox.

tbertels commented 4 years ago

I've tested it and there's two problems:

Other than that, it works perfectly, no wrong tab restore. Of course the previous session closed tabs are only saved for the next session, which should be enough in most cases.

M-Reimer commented 4 years ago

That the tabs are shared after restart is by intention. Way too much trouble to separate them per window. And once a window is closed it would be my job to clean up the now obsolete tabs. It is unlikely that someone really needs closed tabs from an older session and even more unlikely that he still knows in which window he has to restore.

M-Reimer commented 4 years ago

New beta release is up. This fixes the "options" UI and also fixes Android. If there are no additional issues, I'll publish this as 6.0.0 soon.

I also had another look at the "shared with other windows" issue and it is not worth to even try to fix this in a workaround like this. The problem is that restoring a window gives it a new ID. There is no way for an Add-on to keep track of this as Firefox chooses the ID's internally.

tbertels commented 4 years ago

Working great here, looking forward to use it on my main install!

Indeed, having to reimplement the whole Session Restore component wouldn't be much of a workaround.

Kinematics commented 4 years ago

I'm not having any issues, after a little bit of testing. Will give it a day or two to see if anything crops up.

M-Reimer commented 4 years ago

I've uploaded version 6.0.0, now.

While this doesn't actually fix the problem, it should work it around as good as possible. Best case users won't even notice that something has changed. I'll close this issue, now, as there is already a reference to the relevant Mozilla bug in the source code. Once this bug gets fixed, the workaround needs a version check and as soon as the fix lands in the ESR version the workaround can be removed again.

Kinematics commented 4 years ago

Thanks for the work. The only issue I've had over the last week is that closed windows don't have an option to re-open them from the extension, but that's relatively minor.

M-Reimer commented 4 years ago

The problem is that a "closed window" does not provide any useful information with the Mozilla API. It's just a window. No way to have access to the individual tabs inside. If this would be possible, then I would have added some way to restore windows already. But without this the windows just have numbers and no useful information for the user to know which one to restore.

But that's the usual tragedy with Mozilla. You have to be really creative with workarounds to create useful Add-ons for Firefox as the API is full of bugs. I had a bigger XPCOM based Add-on before WebExtensions was added to Firefox and even there it was often needed to work around problems. So this is nothing new.

And even worse: Bugs filed for the WebExtensions API often are open for years without anyone caring too much. Seems like after all Add-ons are pretty low priority.

arturpragacz commented 4 years ago

The problem is that a "closed window" does not provide any useful information with the Mozilla API. It's just a window. No way to have access to the individual tabs inside.

Maybe I'm misunderstanding what you're saying here, but I'm pretty sure the returned 'window' object does have a 'tabs' property, which you can use to read the titles and urls of those tabs.

Off course you can't restore those tabs individually, you can only restore the whole window, but even with that limitation the option to also restore windows, not only tabs, would be nice to have in your extension.

M-Reimer commented 4 years ago

Last time I checked this preference was useless and returned nothing. I also failed to just find some title of any tab for this window. So all I could have shown would be "Window ID $SOMENUMBER" which is not that useful. I don't know if this is fixed in the meantime but at least for the bugs I watch it seems like Addon-API is really really low priority.

arturpragacz commented 4 years ago

Maybe the issue was that you need "tabs" permission for that as well, not just "sessions". Or maybe it was just broken. In any case it seems to be working now.