Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify container switching conditions #105

Conversation

@LoveIsGrief
Copy link
Collaborator

LoveIsGrief commented Oct 10, 2019

The conditions when opening in the default firefox container (NO_CONTAINER in our code) were different to when opening in any other container. We didn't check if the container was changing, so NO_CONTAINER targets would keep closing the old tab and creating a new one despite no actual container change happening.

That was because it was assumed passing a container ID when creating a tab would always open the tab a container. In fact the default container does have an ID, thus we can always pass a container ID to tabs.create.

We now do all the same checks before opening a tab in a new container (including if the container is being switched).

Fixes #102 - Browsing sites bound to No Container open up in new tabs

The conditions when opening in the default firefox container (NO_CONTAINER in our code)
 were different to when in any other container. We didn't check if the container was changing,
 so NO_CONTAINER targets would keep closing the old tab and creating a new one despite no
 actual container change happening.
That was because it was assumed passing a container ID when creating a tab would always open the tab a container. In fact the default container does have an ID, thus we can always pass a container ID.

We now do all the same checks before opening a tab in a new container.

Related to #102 - Browsing sites bound to No Container open up in new tabs
@LoveIsGrief LoveIsGrief added the bug label Oct 10, 2019
defaultCookieStoreId);
}
targetCookieStoreId = defaultContainer.cookieStoreId;
console.debug('Going to open', url, 'in default container', targetCookieStoreId, defaultContainer.name);

This comment has been minimized.

Copy link
@kintesh

kintesh Oct 11, 2019

Owner

Not sure about having logs in the release. Don't want to populate someone's console with our logs.

This comment has been minimized.

Copy link
@LoveIsGrief

LoveIsGrief Oct 11, 2019

Author Collaborator

I commented out the line to be able to comment it in once #96 is implemented

@kintesh kintesh merged commit 69fde9a into kintesh:master Oct 12, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.