Opened 5 years ago

Closed 3 months ago

#2031 closed change (rejected)

Move the logic of disabling/enabling of ABP for the particular web site into ABP core.

Reported by: sergz Assignee:
Priority: Unknown Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: fhd, greiner, oleksandr, erikvold Blocked By:
Blocking: Platform: Unknown
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by sergz)

Background

Each extension has its own peculiarities how to disable or enable ABP for the particular web site. For example, to disable, the extension for Google Chrome finds the exception filter for the current URL and removes that filter. However, so far the extension for IE manually crafts the filter text (@@||www.some-domain.com^$document) and tries to remove that filter. Who knows how it works in Maxthon?
I find the solution in the extension for Chrome to be better because it's closer to the user's intention, he does not care whether the web site is whitelisted by domain or by using another exception filter.

It would be better to have this logic in one place and share among the platforms.

What to change

Add:

  • togglePageWhitelisting(string url) which accepts the URL of the current web site and creates the required filter or removes the corresponding exception filter if it already exists.
  • toggleSiteWhitelisting(string domain) which accepts the domain entered by the user, verifies it, removes "www." prefix if necessary and creates the required filter or removes the corresponding exception filter if it already exists.

This should subsequently be called in platform-specific code instead of manual determining which filters should be removed and instead of manual constructing of the filter text.

Additional references

#1936, #1104, #2070

Change History (8)

comment:1 Changed 5 years ago by greiner

  • Description modified (diff)

I made the "What to change" section a bit more specific. Feel free to change the parts that don't correspond with what you had in mind.

Note that domains are still being whitelisted by manually constructing such a filter so are you also suggesting to introduce a function in the core for whitelisting a domain?

comment:2 Changed 5 years ago by sergz

  • Description modified (diff)

comment:3 Changed 5 years ago by greiner

  • Description modified (diff)

I streamlined the description a bit to make it more consistent with our current naming and also added missing information.

It's still missing the information to which core module those functions should be added. Currently, that appears to be implemented in lib/appSupport.js and lib/ui.js for Firefox and in options.js and popup.js for Chrome/Opera/Safari.

However, retrieving the information whether a page/site is whitelisted would still be platform-specific (e.g. Firefox, Chrome/Opera/Safari). So that would either need to be added to this issue or require a separate issue.

comment:4 Changed 5 years ago by sergz

However, retrieving the information whether a page/site is whitelisted would still be platform-specific (e.g. ​Firefox, ​Chrome/Opera/Safari). So that would either need to be added to this issue or require a separate issue.

Can we use isPageWhitelisted for this purpose?

comment:5 Changed 5 years ago by sergz

  • togglePageWhitelisting(string url) which accepts the URL of the current web site and creates the required filter or removes the corresponding exception filter if it already exists.
  • toggleSiteWhitelisting(string domain) which accepts the domain entered by the user, verifies it, removes "www." prefix if necessary and creates the required filter or removes the corresponding exception filter if it already exists.

I guess, I need to comment the current API because I don't find it good.

I expected that togglePageWhitelisting should behave like toggleSiteWhitelisting, and togglePageWhitelisting is called when the user clicks "Enable on this site"/"Disable on this site", and toggleSiteWhitelisting is used from the settings page where the user operates with the domains. The difference is that togglePageWhitelisting knows how to work with the URL, while toggleSiteWhitelisting works merely with the domain.

I've just discovered that in Firefox we provide a way to disable/enable ABP on the particular page only, so the naming becomes confusing, because it was not expected that togglePageWhitelisting works only for the particular page. May be we need another API.

The idea is that I want to feed the current URL to the engine and let the core to extract all required information from it and create, remove or properly modify the existing filter(s).

comment:6 Changed 5 years ago by sergz

  • Description modified (diff)

comment:7 Changed 12 months ago by erikvold

  • Cc erikvold added
  • Tester set to Unknown
  • Verified working unset

comment:8 Changed 3 months ago by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from new to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Note: See TracTickets for help on using tickets.