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

Cannot re-enable site after disabling #2516

Closed
dhowe opened this issue Feb 24, 2024 · 11 comments
Closed

Cannot re-enable site after disabling #2516

dhowe opened this issue Feb 24, 2024 · 11 comments
Assignees

Comments

@dhowe
Copy link
Owner

dhowe commented Feb 24, 2024

I tried to reproduce the issue when...

Description

It seems impossible to un-disable a site after disabling it: see also #2462

Steps to Reproduce

Disable a site, then try to re-able it

@mneunomne
Copy link
Collaborator

@dhowe how did you disable the page in the first place?

The only case in which I was able to reproduce this error was when a root page of a domain is directly added to the trusted-sites list:

https://www.google.com/

instead of

www.google.com

this causes this makes the "re-enable" fail since it is only the second format is removed, but not the first. It seems that uBlock doesn't have this same issue, will check why and resolve asap.

@dhowe
Copy link
Owner Author

dhowe commented Feb 28, 2024

Here is an example:

  1. Visit https://www.carousell.com.hk/
  2. Click 'Disable' on the menu and refresh page
  3. Try to click 'Disable' again to un-disable, nothing happens [primary bug]

"www.carousell.com.hk" is added to TRUSTED SITES

What should happen:

  1. 'Disable' should change to 'Enable' when disabled=true
  2. You should be able to re-enable a site by pressing 'Enable'
  3. It should be more clear what is happening when you press 'Disable' the first time, and whether you have to press again to select domain or page

@mneunomne
Copy link
Collaborator

@dhowe I just implemented the ability to switch the scope of the disable status, without having to chick again on the enable button.

The UI issue that appears from that, is that within the uBlock structure, we can only access true/false if a page is disabled, but we can't know if the scope of this disable is domain or page-specific. Which makes the UI confusing since when u click on the popup, it doesn't properly shows what is the current scope by which the page is disabled.

For that I see two possible solutions:

  • A) Leave it as it is, but hide the popup-arrow while the page is alreadhy disable, to generate less confusion, and make clear that the scope can't be changed while the page is already disabled.
  • B) Implement a value in the popupData object to check if the page is disabled by page-specific or domain rule.

@dhowe
Copy link
Owner Author

dhowe commented Feb 29, 2024

I remember some discussions about why it wasn't practical to store whether the user had chosen page/domain for every site. So how about this flow: (it may be your A):

  • In 'enabled' state, when 'Disable' icon is pressed it acts as a proper menu, and the user must choose page or domain
  • In 'disabled' state, user sees only 'Enable' - pressing this removes the entry (page or domain) from trusted sites
  • To change from domain to page or vice-versa, user must enable then disable again with the different setting

@mneunomne
Copy link
Collaborator

So I guess this mean that the "popup arrow" needs to dissapear when the website is currently disabled? In order not to confuse the user that it is possible to change the scope of "disable" the website while it is already disabled.

@dhowe
Copy link
Owner Author

dhowe commented Feb 29, 2024

yes, it needs to disappear when the text changes to 'Enable'

@mneunomne
Copy link
Collaborator

Ok, I finally understood why this issue with the disable was happening in cases page-specific disable where the last char of a url ended with a '/':

request.url && (request.url = trimChar(request.url, '/')); // no trailing slash

We are, for some reason, trimming the last char of the url request, which was causing the url saved in trusted sites to be faulty. I'll try to will understand why we do this, and make sure this doesn't happen in such case.

@mneunomne mneunomne mentioned this issue Mar 1, 2024
@mneunomne
Copy link
Collaborator

Created an exception for the trimming character to avoid it happening when toggling the disable button. Also set to hide the arrow when the disable is active AND the popup is not yet closed.

@dhowe
Copy link
Owner Author

dhowe commented Mar 3, 2024

We are, for some reason, trimming the last char of the url request, which was causing the url saved in trusted sites to be faulty. I'll try to will understand why we do this, and make sure this doesn't happen in such case.

I don't remember, but this may have been an attempt to ensure that URL versions (https://site.com and https://site.com/) are treated as the same site/domain. Please verify in your fix that this is still the case

@mneunomne
Copy link
Collaborator

In relation to the disable/enable, yes I have tested and it is working as expected. both https://site.com/ and https://site.com/ are treated as https://site.com/ when disabling as page-specific, and site.com when treating it as domain.

@mneunomne
Copy link
Collaborator

Fixed, closing

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

No branches or pull requests

2 participants