Opened on 04/22/2016 at 12:01:15 AM
Closed on 10/11/2016 at 06:44:58 AM
#3969 closed defect (fixed)
Hits are not counted for custom CSS property rules
| Reported by: | Ross | Assignee: | trev |
|---|---|---|---|
| Priority: | P3 | Milestone: | Adblock-Plus-2.8-for-Firefox |
| Module: | Adblock-Plus-for-Firefox | Keywords: | |
| Cc: | sebastian | Blocked By: | #3999, #4000, #4138 |
| Blocking: | Platform: | Firefox | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by trev)
Environment
ABP 2.7.2.4166
Firefox 45 / Windows 8
How to reproduce
- Install ABP dev build.
- Add the custom filter: testpages.adblockplus.org##.abp-logo
- Navigate to https://testpages.adblockplus.org
- Observe logo is hidden (expected). Observe hit is incremented against rule (expected).
- Disable the .abp-logo filter added in Step 2.
- Add the custom filter: testpages.adblockplus.org##[-abp-properties='width: 64px;']
- Refresh the https://testpages.adblockplus.org tab.
- Observe the logo is hidden (expected). Observe the hit counter for the rule stays at 0 (unexpected).
Observed behaviour
Hits are not counted against CSS property rules.
Expected behaviour
Hits to be counted as with other rules.
Background
As CSS property filters were initially implemented for Chrome, there is no hit counting functionality.
What to change
Update dependency on adblockpluscore to revision f7f491edb1b7 (imports #4000). Update dependency on adblockplusui to revision f0daa0b325ed (imports #3999). Make sure to consider the second parameter in the addSelectorFunc callback - increase hit counts for these filters (unless it's a private browsing window) and add these hits to the list of blockable items.
Hints for testers
The adblockplusui dependency update imports a whole lot of additional changes, mostly related to the new options page. None of these should be relevant for the Firefox build. However, retesting the functionality of the first-run page would be a good idea.
Attachments (0)
Change History (11)
comment:1 Changed on 04/22/2016 at 12:26:29 PM by trev
- Component changed from Adblock-Plus-for-Firefox to Core
- Description modified (diff)
- Priority changed from Unknown to P3
- Ready set
comment:2 Changed on 04/27/2016 at 05:30:13 PM by trev
- Component changed from Core to Adblock-Plus-for-Firefox
comment:3 Changed on 04/27/2016 at 05:33:05 PM by trev
- Blocked By 3999 added
comment:4 Changed on 04/27/2016 at 05:37:04 PM by trev
- Blocked By 4000 added
comment:5 Changed on 04/27/2016 at 06:36:06 PM by trev
- Description modified (diff)
- Owner set to trev
comment:6 Changed on 06/08/2016 at 01:19:38 PM by trev
- Blocked By 4138 added
comment:7 Changed on 06/08/2016 at 01:43:25 PM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:8 Changed on 06/10/2016 at 09:41:59 PM by abpbot
A commit referencing this issue has landed:
Issue 3969 - Hits are not counted for custom CSS property rules
comment:9 Changed on 06/10/2016 at 09:42:38 PM by trev
- Milestone set to Adblock-Plus-for-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:10 Changed on 08/25/2016 at 04:11:16 PM by sebastian
- Cc sebastian added
- Resolution fixed deleted
- Status changed from closed to reopened
If you now use a CSS property filter that is translated into a CSS selector which doesn't match anything (e.g. testpages.adblockplus.org##foo [-abp-properties='width: 64px;']) it is still counted as a filter hit and shows up as "hidden" in blockable items.
Not sure if there is any other way to fix that for Firefox, but also considering the devtools panel on Chrome, I think we'd have to make the addSelectorsFunc take a mapping of selectors to filters.
comment:11 Changed on 10/11/2016 at 06:44:58 AM by trev
- Resolution set to fixed
- Status changed from reopened to closed
What you are proposing requires significant changes to Core. While it does make sense, we should definitely deal with it in a separate issue. I created #4515, the issue here is resolved.
This will require changes to both Core and UI code in addition to Firefox, so let's turn this into a Firefox bug and file the other ones as blockers.