Opened 5 years ago

Closed 2 months ago

#1747 closed change (rejected)

GetElementHidingSelectors should return an empty list if the website is whitelisted by any option

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

Description

Background

Since this question periodically arises it would be nice to discus it to better understand whether we need it or not and the reasons of the final decision.

Currently the client of libadblockplus and of the core library has to firstly test whether the website is whitelisted or not and only then call GetElementHidingSelectors if necessary.

Pros

  • avoiding of the same logic on different clients

Cons

  • the current version seems good from the API fatness point of view, there are no auxiliary parameters, merely what we need, there is no additional logic (see Pros) which can limit the usage of it.
  • the current version seems useful to show potentially blockable items, otherwise the API could be overcomplicated with additional flags/parameters
  • for the performance reason we should not call it twice or more for the same website, at the same time we need to know exactly whether the website is whitelisted or not.

Please leave your thoughts.

Change History (3)

comment:1 Changed 5 years ago by fhd

Note that it's not just GetElementHidingSelectors, it's the same with Matches - the logic has to be in the client currently. If we were to change this, we should change it everywhere IMO, everything else seems confusing. But I suppose you're not a fan of that since you only propose it for GetElementHidingSelectors?

comment:2 Changed 2 months ago by greiner

  • Cc greiner added
  • Component changed from Unknown to Core
  • Tester set to Unknown
  • Verified working unset

comment:3 Changed 2 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.