search

Stigmatoz / web-activity-time-tracker

Chrome Extension that tracks and limits time you spent on sites
https://chrome.google.com/webstore/detail/web-activity-time-tracker/hhfnghjdeddcfegfekjeihfmbjenlomm
MIT License
733 stars 115 forks source link

Fix restrictions #53

Closed tschettler closed 2 years ago

tschettler commented 2 years ago

Fixes two issues with restrictions:

  1. Issue where an added restriction without a path did not match a url with a path.
  2. Race condition loading restriction list and setting restriction icon on charts

Ideally, we would add a restriction icon for urls that are restricted by path as well, maybe this could be a future enhancement.

Stigmatoz commented 2 years ago

Thank you, Travis. Now I have fixed one small error with a missing icon of the current domain.

And now we have the same problem with restrictions. This is reproduced when we set the path in the list of constraints, for example, "github.com ", and then open the page with the URL "github.com/web-activity-time-tracker ". We get the current tab from the storage with the current path, but now we store each tab with different URLs within the current domain. We need to block the site if the current path is nested in a path from the list of restrictions.

Perhaps we need to make a new flag in the list of restrictions indicating "the entire domain" and "Only the current page with all nested paths". This can work more accurately with a list of restrictions.

What do you think ?

tschettler commented 2 years ago

Thanks for the feedback, let me take a look and try to understand the issue, because setting restrictions at the host level should still be working.

tschettler commented 2 years ago

It turns out this was caused by a pre-existing race condition, where currentTab was being cleared before the chart was displayed due to how chrome.windows.getLastFocused works. No need for your latest change so I reverted it. Fixed via #54.

Stigmatoz commented 2 years ago

Thanks for the feedback, let me take a look and try to understand the issue, because setting restrictions at the host level should still be working.

Hi, Travis. We have a restriction error in the method isLimitExceeded(domain, tab). We pass only the current tab to this method. When we open two or more tabs in the current domain with different URLs and different nested paths, we now check only the time for the current tab, and we need to check the time for all tabs with a URL that includes a restriction URL.

tschettler commented 2 years ago

Hi, Travis. We have a restriction error in the method isLimitExceeded(domain, tab). We pass only the current tab to this method. When we open two or more tabs in the current domain with different URLs and different nested paths, we now check only the time for the current tab, and we need to check the time for all tabs with a URL that includes a restriction URL.

Shouldn't it only be checking for the current tab? Maybe I'm not understanding, are you saying the expectation is that the timer will increment for tabs that are not active? So if I have two tabs open for different restricted Urls and I'm only active on one of them, the extension will increment the activity for each tab and ultimately block both, even if I never go to the other tab?