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

Default containers #91

Merged
merged 57 commits into from
Oct 8, 2019
Merged

Default containers #91

merged 57 commits into from
Oct 8, 2019

Conversation

ghost
Copy link

@ghost ghost commented Sep 5, 2019

dev TODO

  • toggable preference group
    Visuable toggle could be done with checkbox and CSS that hides, greys out or deactivates everything in the group
  • create "container name" preference
  • use "container name" preference
  • Allow "No Container" as container name
  • use "container name" preference with variables
  • use "container name" preference with ms variable
  • "rule" preference
  • use "rule" preference
  • use "rule" preference with variables
  • create "lifetime" preference (ChoicePreference with radio buttons)
  • use "lifetime" preference: "Forever"
  • use "lifetime" preference: "Until last tab is closed"
  • check containers at extension startup for lifetime preference
    Those that should've been deleted, will be deleted
  • Grey out preference group when not selected

TODO future PRs

  • Preference group presets

Discussion questions:


This will contain the work on default containers as well as their preferences.

It will introduce the options for default containers described in #10 and #80 (possibly other related issues if I remember/find them again).

What is this "default container" you speak of

The container into which tabs will be placed that cannot be placed into any other container. In other words, if a tab opens or updates its URL to one that cannot be matched by a rule in the extension, it will be put into a default container according to the preferences listed below.

Preferences

Since there have been multiple proposals on what should be done with the default container and they are at times mutually exclusive, it will be up to the user to decide. In order to do so a group of preferences related to the default container will be provided that hopefully cover the requests.

The preference group will have certain presets for the user to reach certain configuration states easily.

Suggested preference group

The title of the group, as seen by the user, will be Default container. The entire group can be toggled on or off. If off, nothing will be done with unmatched URLs. Once activated the following preferences will come into effect:

Container name (string input)

The name the default container will possess. It will be possible to create a dynamic name using certain variables:

  • ms: current time in milliseconds
  • domain: the simple domain without the TLD or anything else
  • fqdn: FQDN
    host will work as an alias
  • tld: TLD

Other variables can be requested (most likely in feature requests).

Examples:

URL containerise_{ms} {domain} {fqdn} default container
https://example.com containerise_1567718601836 example example.com default container
https://old.example.com containerise_1567718628706 example old.example.com default container
https://an.old.example.com containerise_1567718738989 example an.old.example.com default container

Lifetime (choice list)

  • Forever: self-explanatory
  • Until last tab is closed: Once the last tab in the container is closed, the container will be deleted too

A possible countdown e.g "60 minutes" might be possible, but that's out of the scope of this PR.

Rule addition (string input)

This will create an a rule for the container. This is specifically for #80. Containers could thus be automatically created for domains.

Available variables:

  • domain: the simple domain without the TLD or anything else
  • fqdn: FQDN
  • tld: TLD

Examples:

URL .{domain}. *.{domain}.{tld} {fqdn}
https://example.com *.example.* *.example.com example.com
https://old.example.com *.example.* *.example.com old.example.com
https://an.old.example.com *.example.* *.example.com an.old.example.com

Suggested presets for the preference group

"Random, temporary containers"

By default, any unmatched URL would be put into a newly minted, unique, random container that destructs after the last tab in the container is removed.

"Autocreate containers per domain"

Close #80

"Default container"

All unmatched URLs would end up in the same container. If I'm not mistaken this would fix #10

"Default temporary container"

Same as "Default container" with the change that once the last tab in the default container is closed, the default container would be closed. Fear not, the default container would be created again once an unmatched URL is encountered.

This will contain the preferences related to the default container
@ghost ghost self-assigned this Sep 5, 2019
LoveIsGrief added 2 commits September 6, 2019 16:52
Saving hasn't been tested yet!

Additionally the ChoicePreference and StringPreference have been added.

A major change is the introduction of the preferences.json and building
 the preferences UI from a JSON. This allows us to separate the definition
 of preferences from the code as much as possible.

A greater separation of concerns was further introduced using the raw-loader.
It will mark a move away from using the <template> node in index.html
 to a direct import thanks to webpack. This will also help with finding
 the corresponding files for the preference UI more quickly.

A somewhat minor change is the renaming of uiName, ui_title and other
 params to the Preference constructor to simpler and more common names.
Maybe the "ui" prefix will be introduced later on again if it isn't clear
 what the parameter is used for.
@ghost
Copy link
Author

ghost commented Sep 6, 2019

First generation from preferences.json
afbeelding

"Save" doesn't work yet, but that's coming

@ghost ghost requested a review from kintesh September 8, 2019 15:41
@NomDeMorte
Copy link

Having a single global default container carries problems of it's own. See #53 to pick the first example I found.

For example, right now I'm logged into github with two different accounts, in two different containers. If I open an unmatched URL from this container, I'd like it to be opened in the default 'No container', but if I opened that same unmatched URL in the other container, I'd like it to remain in that same container that I'm opening it from. Same URLs in both cases, but different behaviour based on the container.

But, I notice you mention in the UI that the default container name can be a variable. Is this to allow for a future featureset where the variable is stored in the ruleset, this implementing the entry and exit rulesets discussed here?

@ghost
Copy link
Author

ghost commented Sep 9, 2019

Hi @NomDeMorte

Thank you for your response. Your focus is on entry and exit rules. I don't think default containers, as I am going to implement them, will clash with that feature request.

Since you say you're a retired dev, I think a skeleton of some python code will be simpler here (since it's closer to pseudo code). Ignore the camelcase please.

def handleUrlChange(currentContainer, oldUrl, newUrl):
    
    if oldUrl.host == newUrl.host:
        return
    
    # Handle exit rules
    newContainer = currentContainer.exitRules.getContainer(newUrl)
    if newContainer:
        return createTab(newUrl, newContainer)
    
    # Handle entry rules
    newContainer = getContainer(newUrl)
    if newContainer:
        return createTab(newUrl, newContainer)
    
    # Default container being implemented
    newContainer = getDefaultContainer(newUrl)
    if newContainer:
        return createTab(newUrl, newContainer)
    
    # Default container not activated.
    # Nothing to do

I can ping you once I have implemented getDefaultContainer then you'll understand what the naming will do from the code. All it does is generate a name for the container. If you look at the examples in my first comment, you should get an idea.

LoveIsGrief added 4 commits September 9, 2019 12:48
This will be used for `defaultContainer.name` and other preferences.
It will give users to use variables we provide them
Simple strings without any variables shouldn't attempt replacement
 - ContextualIdentities::create added to create containers in a more
   standard manner
 - handle is now directly an async method to use `await buildDefaultContainer`
   without too much indentation
That makes accessing values of return preferences easier.
Before we would get the object stored in the DB which has {key, value}.
@ghost
Copy link
Author

ghost commented Sep 9, 2019

Example of {domain} container name

@NomDeMorte
Copy link

NomDeMorte commented Sep 9, 2019

Your focus is on entry and exit rules.

So is yours :) A global default container is a global exit rule.

If I'm understanding your pseudocode correctly, getDefaultContainer(newUrl) could just call .exitRules.getContainer(newUrl), where .getContainer() (the container class's member function) handles unmatched URLs - as should getContainer() (the global function) .... That being the case, there is no need for getDefaultContainer() to exist really.

It will be good if your implementation doesn't directly block other related feature requests... but it would also be a shame for your work to have to be removed and replaced with something else, in order to meet those requests, when the same amount of your work could instead apply some feature requests now (ie global exit rules), and have half of the work done for other feature requests (ie per-container exit rules).

I just want to make sure you are getting the best return on your investment of time, and also allowing future work to benefit from that investment.

LoveIsGrief added 3 commits September 10, 2019 13:07
This is preparation for their lifetime preference
Hide toggle checkbox for PreferenceGroups if they aren't toggleable

Try and make groups visually distinct from preferences.
Still looks like ass, but hopefully somebody will improve that
@ghost
Copy link
Author

ghost commented Sep 13, 2019

Cheers. I'll try to recreate it.

@crssi
Copy link

crssi commented Sep 13, 2019

What about save on change and no button at all?

Cheers

@ghost ghost mentioned this pull request Sep 15, 2019
@ghost
Copy link
Author

ghost commented Sep 15, 2019

I think I'll go for the "save on change". It might be more intuitive indeed

@NomDeMorte
Copy link

Save on change is bad because if a user clicks something in error it's done. That's why Save/apply/close/etc buttons are standard.

LoveIsGrief added 2 commits September 16, 2019 17:07
Each preference can trigger a change event. That's only triggered if an actual
 change is made that's different from the current db value

Preference groups trigger change events as well as 'childChange' events.

The ContainerPreferenceGroup has multiple PreferenceGroup children and
 their childChange events are 'bubbled' / forwarded
"No Container" can't be removed

ContextualIdentity.get(NO_CONTAINER.cookieStoreId) returns a list
 instead of a single container to adhere to the return type
@kristofferR
Copy link

Thanks for implementing support for No container, but unfortunately it's really wonky. It seems to still use the container functionality despite it's not being needed. The pages are quickly closed and reloaded, and the back button/history doesn't work at all.

Would it be possible for it to instead completely bypass the container functionality so that pages that open without a container function as normal?

LoveIsGrief added 4 commits September 16, 2019 20:13
The browser.storage can return undefined for non-existing items
When already without a container we kept recreating the tab believing
 it wasn't without a container. We now check beforehand.

Tried to make it more readable by using variable names for the booleans
'firefox-default' is now used instead of 'no-container' since that's what
 firefox actually uses to designate that no container is being used
@ghost
Copy link
Author

ghost commented Sep 16, 2019

@kristofferR It should work now.

@ghost ghost deleted a comment from NomDeMorte Sep 17, 2019
LoveIsGrief added 4 commits September 17, 2019 11:34
This is now done in a listener instead of only when triggered manually
 by our code. It's possible for containers to be deleted externally
 e.g other extensions or by the user in the firefox preferences
When testing whether temporary containers are deleted at the start of firefox
 a "permanent" profile is useful
If there's a bug this should help mitigate storage getting filled up
@crestcere
Copy link

Hey, out of context but I want to try this. Can you publish compiled version of this branch? I want to use it.
Thank you.

@ghost
Copy link
Author

ghost commented Oct 2, 2019

@sungerbob this is the one I'm currently using containerise-3.5.3alpha.zip. I uploaded it to IPFS. If it goes down I'll find another host. It could be interesting to add builds to the CI 🤔 (but that would be in another PR)

LoveIsGrief added 3 commits October 3, 2019 11:59
It is one that doesn't have any tabs anymore and should've been deleted
@kintesh kintesh merged commit b3edd11 into kintesh:master Oct 8, 2019
@ghost ghost deleted the default_container branch April 15, 2020 16:02
@ghost ghost restored the default_container branch June 7, 2020 12:34
@ghost ghost deleted the default_container branch March 6, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants