-
Notifications
You must be signed in to change notification settings - Fork 349
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
Associate userContextId=0 with default container (fix can't open in default container if url assigned to a container #1312) #1599
base: main
Are you sure you want to change the base?
Conversation
b8ad4e8
to
347a057
Compare
@garywill is this causing the code to do anything different or console errors? My understanding is properties being undefined should use the |
src/js/background/assignManager.js
Outdated
@@ -490,6 +490,9 @@ const assignManager = { | |||
currentCookieStoreId = backgroundLogic.cookieStoreId(currentUserContextId); | |||
confirmUrl += `¤tCookieStoreId=${currentCookieStoreId}`; | |||
} | |||
else { | |||
currentCookieStoreId = "firefox-default"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my other comments however if we need this I would prefer all non test usage has a global.
Yes it is causing browser doing different. No console warning or error is showed. And there's some odd behavior still confusing me. If leave the variable undefined :
It's the second one that is undesired and giving user unwanted container. I have no idea why this happend. These behaviors can be observed by adding |
Some time ago this addon could be set as allowed in private window. But in private window user can't open any container tab, browser console shows After |
associate userContextId=0 with CookieStoreId="firefox-default"
According to Firefox source code, default container has https://searchfox.org/mozilla-central/source/browser/base/content/utilityOverlay.js#776 in function // If we are excluding a userContextId, we want to add a 'no-container' item.
if (excludeUserContextId || showDefaultTab) {
let menuitem = document.createXULElement("menuitem");
menuitem.setAttribute("data-usercontextid", "0");
....... Addon's background logic didn't associate 0 with default container.
I also did some simple test (manually, with eyes) on some cases needing reopening. When opening |
Background is #1312
After debugging I found
browser.tabs.onCreated
is called twice.The first call is ok.
tab.cookieStoreId = firefox-default
.But the second call
tab.cookieStoreId = firefox-container-x
. The container id is not desired.Then after digging the code, found in
reloadPageInContainer()
variablelet currentCookieStoreId;
is left undefined ifcurrentUserContextId
is false. That caused unwanted behavior.