-
Notifications
You must be signed in to change notification settings - Fork 350
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
Add allowlist for isolated containers #2033
base: main
Are you sure you want to change the base?
Conversation
This implements a per-container allow-list that is applied to isolated containers. This can allow multiple containers to have an overlapping set of sites that they are isolated to. For example: Container A and Container B could both be isolated and have calendar.example.com in their allow list. Both Container A and Container B would then be able to visit calendar.example.com. See: mozilla#1892 and mozilla#1887 for additional motivation.
This looks like exactly what I need in order to use different containers for different google products:
|
@mikeyhew You can pull the source from this PR and load it as a temporary extension https://extensionworkshop.com/documentation/develop/temporary-installation-in-firefox/ to test the UX and provide feedback on whether this works for your use-case. I would imagine it would, because I use something similar (I have a container for each separate Google Account, but that has a similar need to overlap for certain domains like accounts.google.com) |
@ncallaway thanks, I installed it temporarily and it worked, and now I've installed it more permanently to use during my day-to-day. It's helpful that you can add sites manually, because sometimes it redirects away from the site you want to add before you can add it. I wish there was a way to add "always open in" sites for containers manually too, instead of having to visit the site first. I have noticed an issue though where sometimes it doesn't show any of the UI you added, and sometimes it does. Initially I thought it somehow had installed the unmodified version of the app, but then all of a sudden the allowed sites section showed up. One case where it pretty consistently doesn't show up is when there are no sites assigned to the container yet - when I click on "Manage Site List" link on the page where you edit the container, I get this: Of course, I said that it pretty consistently does that, but I tried it again just now and for the first time, it did show the UI you added even though there were no sites added to the container yet. |
Thanks for this @ncallaway, sorry for the slow response. I'm running this now and it's exactly what I was looking for with #1892, so that's perfect. This change really makes MAC much more usable for me. I would agree with @mikeyhew that my first impression is it was a little tricky trying to set up sites with redirects. In my case I found it tricky adding gmail, since I usually type gmail.com which redirects to mail.google.com and then (for a fresh container) to accounts.google.com, so I had to keep an eye on the address bar and add all those to make it work. I did get it set up and working how I wanted without too much hassle though. I'm going to run this for a while and I'll hopefully have some better feedback. |
This has been working great for me over the last couple of months. The only feedback I have is there are a few spots where the UI looks unstyled and as I mentioned it can be tricky to add pages that redirect, which makes it a little bit of extra work for using with e.g. Google products. Otherwise it's exactly what I wanted from MAC. |
@ncallaway this has been working great for me and I'd love to see it merged. Not really familiar with the review process here, nobody seems to have dropped in to review it. Do you know if we need to raise it with someone? |
@gpoole I'd love to get it merged in as well. I'm not sure exactly what the review and approval process is for contributions. I'm happy to take on the work remaining to get this ready to merge (resolving conflicts, addressing any outstanding bugs like that reported by @mikeyhew, etc). I just want to make sure that it's on a path to being accepted and merged before I finish it up. Do you know who the appropriate reviewers are that can approve merging something in? I'd love to get a final set of feedback and issues to resolve before picking this up and finishing it. |
Would love to see this get merged. |
This implements a per-container allow-list that is applied to isolated
containers. This can allow multiple containers to have an overlapping set of
sites that they are isolated to.
For example: Container A and Container B could both be isolated and have
calendar.example.com in their allow list. Both Container A and Container B
would then be able to visit calendar.example.com.
See: #1892 and #1887 for additional motivation.
Changes
isolated
isolated
onBeforeRequest
logic to account for the container's allow listcontainer-allowin-16
Screenshots
Always Open This Site in...
screenshotUpdated
Manage Site List
screenshotIcon
Demo
demo.mp4
Draft
This is a draft until an icon has been made for the "Allow Opening This Site in...". I'll work on creating that icon later this week.License
I agree to license this code under the project's open source license (MPL 2.0). The new image (
container-allowin-16
) was authored by me and I agree to also license under MPL 2.0