Opened on 11/08/2017 at 06:12:39 PM
Closed on 02/21/2018 at 02:14:44 PM
Last modified on 03/27/2018 at 12:53:09 PM
#6009 closed defect (fixed)
Whitelisting (via options) fails in ABP 3.0
| Reported by: | mapx | Assignee: | agiammarchi |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | User-Interface | Keywords: | |
| Cc: | greiner, saroyanm | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by saroyanm)
Environment
windows 10
ABP 3.0.0.1913beta
FF 57
How to reproduce
go into options
Whitelisted websites
add website: https://www.google.co.uk/
Observed behaviour
Nothing happens, the domain is not whitelisted
(if I add only the domain google.co.uk ABP will work correctly)
Expected behaviour
Like the other method (while on the page / domain to be whitelisted) via clicking ABP icon and then click "Enabled on this site" ABP should extract the domain and add it to the whitelist
..or only the label (ADD WEBSITE => ADD DOMAIN) is wrong. However, would be better having the same behaviour as in the second case.
Specification
Attachments (0)
Change History (16)
comment:2 Changed on 11/08/2017 at 06:15:46 PM by mapx
- Cc greiner added
comment:3 Changed on 11/08/2017 at 06:38:49 PM by trev
- Component changed from Adblock-Plus-for-Firefox to User-Interface
comment:4 Changed on 11/08/2017 at 06:41:05 PM by trev
- Cc saroyanm added; sebastian mjethani removed
comment:6 in reply to: ↑ 5 Changed on 11/08/2017 at 09:14:53 PM by greiner
Replying to trev:
The suggestion sounds reasonable to me. The logic here could detect invalid host names (at the very least, anything with slashes in it) and attempt to interpret them as full URLs (via new URL()). We need to show some kind of error message if it isn't a valid URL or host name is empty.
In the spec we haven't defined what that validation encompasses since we simply wanted to reflect the previous behavior as closely as possible which barely does any validation.
Note that there is already some error handling in place but it doesn't receive any errors so we first have to further look into where/why things are misbehaving.
Changing the spec to include normalization is definitely something we can do to improve this feature (I created a spec ticket for that) but for now the goal is to restore the currently specified behavior.
comment:7 follow-up: ↓ 8 Changed on 11/09/2017 at 10:15:36 AM by mapx
Another bug somehow very related:
As I said above when you add to the whitelisted section a normal link (not only the domain) like this 1:
http://edition.cnn.com/2017/11/08/entertainment/all-the-money-in-the-world-kevin-spacey-replaced/index.html
The filter does not disappear but you'll find it under your custom filters (as happens also in chrome).
The fact is the resulting filter is wrong !
The bug is in ABP for chrome too.
You'll see:
@@||http://edition.cnn.com/2017/11/08/entertainment/all-the-money-in-the-world-kevin-spacey-replaced/index.html^$document
Correct filter:
@@|http://edition.cnn.com/2017/11/08/entertainment/all-the-money-in-the-world-kevin-spacey-replaced/index.html^$document
There is a positive part: you could integrate this as the lost function from ABP legacy ("disable only on this page" (the filter produced this way is exactly that for the lost function)
comment:8 in reply to: ↑ 7 Changed on 11/09/2017 at 01:10:07 PM by greiner
Replying to mapx:
There is a positive part: you could integrate this as the lost function from ABP legacy ("disable only on this page" (the filter produced this way is exactly that for the lost function)
I think you're right. We could make this feature quite a bit more powerful so I added it to the spec ticket.
comment:9 Changed on 12/21/2017 at 11:26:28 AM by fhd
- Cc trev removed
comment:10 Changed on 02/09/2018 at 10:20:50 AM by agiammarchi
- Owner set to agiammarchi
comment:11 Changed on 02/09/2018 at 10:20:56 AM by agiammarchi
- Review URL(s) modified (diff)
comment:12 Changed on 02/09/2018 at 10:21:12 AM by agiammarchi
I am following these specifications: https://gitlab.com/eyeo/spec/issues/105
comment:13 Changed on 02/14/2018 at 03:51:05 PM by saroyanm
- Description modified (diff)
- Priority changed from Unknown to P3
- Ready set
Added the specification merge request.
comment:14 Changed on 02/14/2018 at 04:40:35 PM by abpbot
A commit referencing this issue has landed:
Issue 6009 - Convert URLs into domains when adding to whitelist
comment:15 Changed on 02/21/2018 at 02:14:44 PM by saroyanm
- Resolution set to fixed
- Status changed from new to closed
comment:16 Changed on 03/27/2018 at 12:53:09 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed. URLs are converted into domains when added via the whitelist tab.
ABP 3.0.2.1998
Firefox 51 / 58 / Windows 10
Chrome 49 / 65 / Windows 7
Opera 36 / 51 / Windows 7
The suggestion sounds reasonable to me. The logic here could detect invalid host names (at the very least, anything with slashes in it) and attempt to interpret them as full URLs (via new URL()). We need to show some kind of error message if it isn't a valid URL or host name is empty.