Skip to content

Comments

URL parsing refactor to support paths#52

Merged
Stigmatoz merged 4 commits intoStigmatoz:masterfrom
tschettler:support-paths
Oct 8, 2021
Merged

URL parsing refactor to support paths#52
Stigmatoz merged 4 commits intoStigmatoz:masterfrom
tschettler:support-paths

Conversation

@tschettler
Copy link
Contributor

This update refactors URL parsing logic into a separate url class that can be used for comparing both the domain and the path. This allows for setting restrictions by path and domain. For example, one may want to set a restriction for a URL such as bing.com/fun, while still allowing unrestricted access to the rest of the bing.com domain.

image

Adding a restriction for a domain will also restrict subdomain. For example, using the following restriction:
image

The block page will appear for subdomains as well:
image

Also fixes a minor off-by-one date parsing issue on the date range.
Before:
image

After:
image

Open for discussion on this PR, please let me know if there are any issues.

Closes #21, without requiring an asterisk. Also opens up the door for some other feature requests to be implemented more easily (#3, #25, #32, #43, #47).

Thanks in advance for your consideration!

Travis

@Stigmatoz
Copy link
Owner

Hi, Travis.
Thanks for your commits. But we have a problem. In the list of sites, we repeatedly see the same domain.
We can do the grouping (as suggested by #47) and then we will see the main domain with an expanded list of nested paths.
This is one of the options. We have to think about how to do it better.

image

p.s. Sorry for my english)

@tschettler
Copy link
Contributor Author

The domain groupings and nested paths is a good idea, but maybe save that for a future change. I'll take a look at fixing it to make sure at least the domains are grouped, thanks for the great feedback!

And no worries, your English is excellent!

@tschettler
Copy link
Contributor Author

Please take another look. This does not include grouping of subdomains, but should fix the regression.

@Stigmatoz
Copy link
Owner

Thanks. These are nice changes for the next grouping function. We now see different times for each tab in the domain.

@Stigmatoz Stigmatoz merged commit 2ad18c9 into Stigmatoz:master Oct 8, 2021
@Stigmatoz
Copy link
Owner

What we have now.

  • Restrictions do not work when we specify only the domain in the list of restrictions.
  • We have lost the icon with restrictions in the list of sites.
    image

@tschettler tschettler deleted the support-paths branch October 8, 2021 16:43
@tschettler
Copy link
Contributor Author

tschettler commented Oct 8, 2021

  • Restrictions do not work when we specify only the domain in the list of restrictions.

This does work for me, but I think I may see an issue here and will get it fixed up.

  • We have lost the icon with restrictions in the list of sites.

There's a pre-existing race condition on the restriction list when it is initially loaded. You can get the icon if you navigate to another chart and then back. I will work on getting this resolved as well.

@tschettler
Copy link
Contributor Author

These issues should be fixed with #53.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Domain masks

2 participants