Opened on 11/01/2015 at 12:13:52 PM
Closed on 11/19/2015 at 06:09:26 PM
#3251 closed change (fixed)
Save data from element hiding hits again
| Reported by: | trev | Assignee: | trev |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-2.7-for-Firefox |
| Module: | Adblock-Plus-for-Firefox | Keywords: | e10s 2015q4 |
| Cc: | Blocked By: | ||
| Blocking: | #3208 | Platform: | Firefox |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
https://codereview.adblockplus.org/29329742/ |
||
Description
Background
With the changes from #3208 the elemHide module will call Policy.shouldAllow() in order to check whether a hiding filter should be active. However, only the allow key of the response is considered, the hits key is ignored and the data isn't stored.
What to change
Have the child/elemHide module call shouldAllow from the child/contentPolicy module instead of using its own message to communicate with the parent. The key should be sent as location parameter and resolving it into a filter should happen in Policy.shouldAllow().
Ideally, we should add a parameter callback to Policy.shouldAllow() in order to allow it being called in an asynchronous way (with async message passing). This will require a proper abstraction for responses, similar to the way it was implemented in ext/common.
Attachments (0)
Change History (5)
comment:1 Changed on 11/04/2015 at 02:37:23 PM by trev
- Owner set to trev
comment:2 Changed on 11/04/2015 at 03:06:19 PM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 11/09/2015 at 02:59:23 PM by mario
- Keywords 2015q4 added
comment:4 Changed on 11/16/2015 at 12:56:25 PM by trev
comment:5 Changed on 11/19/2015 at 06:09:26 PM by trev
- Milestone set to Adblock-Plus-for-Firefox-next
- Platform changed from Unknown / Cross platform to Firefox
- Resolution set to fixed
- Status changed from reviewing to closed
The remaining parts landed on branch as well:
https://hg.adblockplus.org/adblockplus/rev/2720407777f5
https://hg.adblockplus.org/adblockplus/rev/0542ba7f5a57
First part landed on branch: https://hg.adblockplus.org/adblockplus/rev/3d7a22037cca