search

sagitta1618 / temporaryTabContainer

Firefox extension that opens each new tab in a new container
https://addons.mozilla.org/en-US/firefox/addon/temporarytabcontainer/
9 stars 1 forks source link

Request: Auto-delete temporary containers per session #2

Closed EstherMoellman closed 4 years ago

EstherMoellman commented 4 years ago

Hi @sagitta1618 , it's been a while since past February, here I'm again, today please with a new request.

The problem is that your add-on (sometimes) doesn't delete temporary containers. I can see same temporary containers accumulating and accumulating over the time, and your add-on is opening temporary containers over these same undeleted temporary containers.

I'm using your add-on with this other add-on: https://addons.mozilla.org/en-US/firefox/addon/tabs-to-bookmark-groups/ I'm not an expert, but from my ignorance, I don't believe TabsToBookmarkGroups add-on is the culprit, because it works well with other temporary containers.

Anyway, please my request is very simple:

1) During same browser session, always keep same temporary container, until a new tab is opened with same url or different url.

2) When browser session is finished and browser closes, all temporary containers are deleted. Every new browser session should have zero saved temporary containers. By the way, that's the meaning of "temporary"... it'll be alive, until browser closes.

Hope you can do it! It'll be fantastic. I still prefer your add-on over the others, because your add-on is lightweight and works very nice for me. Thousand thanks to you in advance!

Big hug! : )

sagitta1618 commented 4 years ago

Hi @EstherMoellman,

Thank you for your feedback. Indeed the containers should be deleted when the browser closes but they are not. I need to fix that. That's something I didn't notice before. Of interest, I also found this extension that does temporary containers on steroid: https://addons.mozilla.org/en-US/firefox/addon/temporary-containers/ I think it's quite good and might fit your needs. I'll try to fix this issue as it's not the behavior I am expecting anyway.

Regards :)

EstherMoellman commented 4 years ago

Hi @sagitta1618 ! Thanks for your replay.

Yes, I know the add-on you mentioned, and I also know the Dev. It's a great add-on, and the Dev also is a great person, (and he has deep knowledge on containers). However, his add-on is too much for me. I don't need all the options it offers. And it consumes almost 1MB RAM, CPU etc. Time ago I opened a request at GitHub, and I asked the Dev to build a lite version of his add-on. But very kindly he said he wasn't interested on that.

And then your add-on appeared. And comparing to all the other similar add-ons over there, your add-on is the most lightweight. And it's perfect for me. Honestly, I don't need more than your add-on! I just ask you, please, to try to include the 2 requests I posted here days ago, related to fix the temporary container auto-delete after every session.

If I can help you... please count on me!

Big hug and have a nice week! : )

sagitta1618 commented 4 years ago

Ok fair enough :)

I pushed an update to v1.0.2 which fix the issue of deleting temporary container. It was caused because I was looking for container names 'priv9' instead of 'TC' to be deleted. This is now fixed.

There still remains issues I need to fix: namely the last open container is not closed as the promise returned on tab closing states that the tab is still using the context. That's something I need to fix in the future as well as general tyding and adding an nicer icon.

Have a good week too!

EstherMoellman commented 4 years ago

... sweet! Thank you for your v1.0.2. I confirm, at my first tests, now temporary containers seem to be deleted. Very cool! But if you're still motivated, then these two are going to be the next great challenges to include into your add-on:

  1. During same browser session, always keep same temporary container for each tab, until a new tab is opened with same url or different url. Please let me explain: At current behavior, in same browser session: a) If I have "url X" tab opened, then I have TC1 (this is perfect); b) If I don't close "url X" tab (TC1), and I open same "url X" in a new tab, then I have TC2 (this also is perfect); c) If I close "url X" tab (TC1), then TC1 is deleted (perfect), and the same with TC2 (also perfect)... however, if TC1 and TC2 are deleted, no "url X" tab is opened, and I open again a new tab with "url X", in same browser session this temporary container should be always the same, only deleted when browser session is finished (browser closing). Why is this useful? Because in same browser session, I don't want to login to same "url X" tab every time I close this unique "url X" tab. And this is going to be needed when your add-on is used with another tab group add-on or tab manager add-on. These kind of tab manager add-ons use to group tabs by hiding or discarding or closing other tabs. The conflict with your add-on and these tab manager add-ons appears every time these tab manager add-ons hide/close/discard a tab... and in same browser session your add-on always will open a new temporary container for same tab. In my example above, if I'm logged into "url X" tab (TC1), but with the tab manager add-on I change tab group, and "url X" tab is closed/discarded/hidden... your add-on always will reopen the same "url X" tab in a TC2, and I always will need to login again and again and again same "url X" tab every time I change tab group. Apologies for my confusing explanation, hope you'll understand what I'm trying to explain. In brief, it'll be great in same browser session, always to keep same temporary container for "url X" tab, until in same browser session a new "url X" tab is opened, or until a different "url Y" is opened, or until browser is closed (browser session ended). This will allow users to change tab groups, but to preserve logins on same temporary containers in same browser session. PS: This is the TabGroup add-on I'm using.

  2. If browser is closed (browser session ended)... every temporary container always should be deleted. This includes the case you mentioned about the last open container. Long time ago this was a bug in Firefox' browser, and all temporary containers weren't deleted, it was an issue affecting all temporary container add-ons. At that time, I remember that some Devs solved the issue simply by deleting every temporary container at browser startup. Their add-ons were deleting every temporary container, every time the browser was launched. The related Firefox' bug was already solved. But perhaps your add-on can use the same old workaround, simply by deleting every temporary container every time the browser is launched? The tab should not be deleted, just the temporary container.

Zillions of thanks in advance! : )

sagitta1618 commented 4 years ago

I've just pushed v1.1.0 that solves the issue with remaining container (request (2)). I actually did just that, cleaning all 'TC' on startup.

Concerning (1), I downloaded the TabGroup extension and I see why this is annoying. I think it's because the TabGroup extension doesn't hide tabs (it saves memory) but rather close and then open them back. From what I understand, the behavior you described is more like a 'persistent container for the browsing session'. While what I had in mind is more a 'tab specific container' that get destroyed once there is no more tabs using it (which happen when you switch group in the TabGroup addons).

I see a few ways to go forward with that but I am not sure I will be able to implement them as my javascript skills are very limited.

Can you let me know your thoughts about that? :)

sagitta1618 commented 4 years ago

Also what to do if there are multiple URL for the same container (multiple tab using the same container). Which URL to join with the TC?

EstherMoellman commented 4 years ago

... I haven't tested yet the latest v1.1.0, but loved to know that the "cleaning all TC on startup" was implemented... T-H-A-N-K Y-O-U! You're doing an amazing job. Congrats!

Now, with regards to (1), yeah, your concept of "a type of persistent container for the browser session" is the perfect description for tab manager add-ons needs. You're right, these kind of tab manager add-ons need a hybrid container, which survives the whole browser session under same tab, even if the tab is closed/hidden/discarded, until browser closes.

The first option you suggested: "Keep all TC alive during the entire browser session"... seems to me the chosen one! I'm not an expert, but seems to me you're in the right path. If I well understand you, this option will prevent to delete all TC in same session and in same tab. And considering your latest v1.1.0 solution, there is no risk to see same TCs in the next browser session, because all them are going to be deleted on browser startup. So what to say? SOUNDS PERFECT! : )

The second option you suggested... hummmmm... well, if the first option works, then honestly I don't see necessity for this your second option (unless I'm missing something). In fact, this second option at first glance and based on my ignorance, seems to me to be problematic, because for example if user wants to login to "url X" in two tabs (same url but in two different tabs), this might be a problem if containers are associated to specific urls... same url will always open in same container, and this is not desirable because we always want every tab in a different temporary container. This issue is similar to what you asked me in your last message (multiple URL for the same container - multiple tab using the same container etc). In brief, to associate urls to containers, seems to me problematic. Oh yeah, I'm sure you're capable to find all the solutions to all the possible problems, but my point here is that I still don't see the purpose of this second option (am I missing something?). Again, your first option seems to me clean and enough, not perfect, but quiet enough to solve the issue (in a simple an elegant solution). Please, correct me if I'm wrong or missing something.

Your third option may or may not create a contradiction... it'll depend on your definition of "no more tabs are using it". For example, the tab group add-on I'm using (TabsToBookmarkGroups), as you well said, it closes tabs when tab groups are changed. So, if your add-on interprets that as "no more tabs are using the container", this may contradict your first option ("keep same container alive during the whole browser session on same tab"). But in the case of TabsToBookmarkGroups, I believe a workaround can be done by looking at the bookmarks created by this add-on. And your add-on may create a temporary container associated to the existence of each bookmark, TC that will survive the whole browser session, until the bookmark is deleted, or until browser closes. It's not going to be a container associated to an url, but instead, it's going to be a container associated to an existent bookmark. If you can build something like that... this will be ultra-mega-cool... the perfect solution. I know the Dev of TabToBookmarkGroups, I worked with him in the creation of his add-on, he's a great guy, and if you need help with the script, we can invite him to give us a hand. I don't worry about other tab manager add-ons, because your add-on works well with hiding/discarding tabs. The only method that has conflict with your add-on, is when the tab is closed, the method used by TabToBookmarkGroups add-on. But as I say and as you know, this add-on creates bookmarks. And the trick will be to associate temporary containers not to urls, but to each bookmark created by this TabToBookmarkGroups add-on. If the bookmark exists, then the TC will survive and remain the same only for this bookmark... until browser closes... and then everything will be deleted on next browser startup. SOUNDS TO ME PERFECT! What are your thoughts? If this works, then will be a solution to every tab manager add-on working with closed tabs. Lot of tab session manager add-ons are based on this closed tabs method, and all these add-ons will be capable to use this solution, having temporary containers with your add-on.

sagitta1618 commented 4 years ago

Can you give this a try? It is based on the 'containerSession' branch of this repo and uses the principle: each URL has a container. I tried it with TabGroup and mail.outlook.com and it kept me logged in while I was changing group tab. Unzip the file and drag and drop the .xpi into firefox to install the extension. If it works fine, I'll publish it on AMO. tempcontainer-1.0.1-an+fx.zip

EstherMoellman commented 4 years ago

@sagitta1618 , of course I can give this a try, it's a pleasure for me, count on me! Here are my first test impressions:

POSITIVE RESULTS: 1) Old TCs are perfectly deleted on session ended/new session. 2) The add-on remembers TCs, even when tab-groups are changed. THIS IS A HUGE PROGRESS! Congrats! Excellent job. By the way, the same perfect behavior happens on logged urls, the add-on remembers the TC and keeps the logged url, even when tab-groups are changed... F-A-N-T-A-S-T-I-C! Beautiful!

LESS POSITIVE RESULTS 1) Sometimes (not always), if the same url already exists in more than one tab in same tab-group... the add-on fails to create a TC. For example: At tab "Group 01", having two tabs with Facebook, the add-on fails to create TCs. 2) Sometimes (not always), if the same url already exists in more than one tab in different tab-groups... the add-on not just fails to create a TC, but also the tab keeps itself flickering/blinking, like trying to create a new tab with the TC but failing on doing that. The only way to stop this bizarre behavior is by closing and reopening the browser. For example: At tab "Group 01" having a Facebook tab, and another tab "Group 02" also having a Facebook tab, the issue appears (sometimes, not always). 3) Even if an url is unique (if it only exists in one tab and in one tab-group), the add-on creates TC the first time the tab is selected, but when browser is closed and reopened, the add-on fails to create TC in same selected tab. Only when the tab is totally closed (deleted from the tab-group), the browser closes and reopens, and the add-on can create TC in the new tab. 4) In same tab-group, if an url has a TC, and I open same url in a new tab, the add-on uses the same TC for the same url (undesirable behavior). For example, at tab "Group 01" I create a Facebook tab and log-in with "X" username, and if I create another new tab with Facebook, the add-on uses the already Facebook logged tab (same TC). The same happens if I create a Facebook tab in tab "Group 01", and another Facebook tab at "Group 02", the second Facebook tab will have same TC than the first Facebook tab, and both will have the same logged username. The ideal here will be always to have different TCs for each tab, even for same urls.

sagitta1618 commented 4 years ago

That's great :) I noticed issue 2 during my tests but couldn't reproduce it all the time. I think it's due to a race in promise and listener event. Issues 1 and 4 are unfortunately inherent to the design choise. The problem is that because TabToBookmarkGroups extension only stores the URLs (and no information about tabId or cookiedStoreId), it's impossible to know to which container each URL belongs to. Or at least I don't see a solution right now for it. Maybe digging more inside how TabToBookmarkGroups stores the informations or maybe match the containers based on their positions in the group tab instead of their URL (might be quite challenging as very asynchrone). or maybe a mix of both, adding a 'TC01' at the end of the saved bookmark to remember in which context to open it -> will need to fork TabToBookmarkGroups for that. Anyway there if food for thoughts.

EstherMoellman commented 4 years ago

@sagitta1618 , as I said, I believe you did a huge progress. The latest v1.1.0 is already tremendously beautiful and works like a charm. And the latest "tempcontainer-1.0.1-an+fx.zip" is the cherry on the top... but IMO it's not ready for AMO. I don't know if it'll will be possible to solve the 4 negative issues, but let's try to solve them. Please, let me send a message to TabsToBookmarkGroups' Dev. Let's see if he has suggestions for your add-on, or perhaps he can make changes to TabsToBookmarkGroups add-on.

jingyu9575 commented 4 years ago

Hi @sagitta1618 , I'm the developer of that TabToBookmarkGroups extension.

My extension already remembers the container id, and when switching groups, it will try to use the original container. Try these steps:

  1. Install both extension and open some tabs / groups. Let's say the current tab is now in the container TC6.
  2. Use about:config to disable tabContainer.
  3. Switch to another group and switch back.

In my test, the same tab will reopen in TC6.

I think the original issue is caused by the group deletion behavior. If tabContainer is enabled, TC6 will be deleted when the group becomes inactive (and all tabs in TC6 are closed). When switching back, TC6 no longer exists and the two extensions settle on a new TC container.

In my opinion, the least intrusive way to fix this is adding this option:

delete all TC on next startup and stop deleting TC when a tab is closed (that will keep all TC alive during the entire browsing session)

This way my extension can reuse the same container in one session and there is no need to implement cross-extension communication.

sagitta1618 commented 4 years ago

Thank you @jingyu9575 for your insights. In the 'sessionContainer' branch, I updated the addon to only delete TC on next startup (so not like the tabContainer extension on AMO does). I also changed the listener so that it only gets fired when a new webrequest is created from a cookieStoredId == 'firefox-default' (the default container). @EstherMoellman can you give it a try? tempcontainer-1.0.2-fx.zip

EstherMoellman commented 4 years ago

Congrats! I couldn't find any issues. At first glance, this version seems to work like a charm. Everything seems to work just perfect. Tomorrow I will do more aggressive tests, but I already passed the last hour doing very nasty things against your latest version, and it seems solid as a rock, couldn't find a single problem.

Beautiful and amazing job @sagitta1618 ! Kudos and thank you!

PS: I suggest you to present this two new features at your AMO' page: 1) Temporary containers are deleted at every new browser session 2) If a tab remains opened, then TC are kept during same browser session (also logged tabs are kept), making this perfect for tab-manager add-ons (tab-groups, tab-sessions etc). This is the only TC add-on fully compatible with tab-manager add-ons (which currently are a strong tendency in browsers). This also is the more lightweight TC add-on.

sagitta1618 commented 4 years ago

Great! I've updated the AMO page with a new version and added explanations and screenshots. Thank you very much for the feedback on this :)