Opened on 03/09/2016 at 08:29:40 AM
Closed on 03/09/2016 at 06:13:02 PM
Last modified on 04/20/2016 at 02:15:06 AM
#3763 closed change (fixed)
Get rid of redundant "add-filters" message type
| Reported by: | sebastian | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P3 | Milestone: | Adblock-Plus-1.12-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | Blocked By: | #3761 | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by sebastian)
Background
The add-filters message type is used by the "Block element" dialog in order to validate a newline-separated block of filters using the filterValidation module, and saves the filters if all of them are valid, responding with an error otherwise.
This logic was mostly redundant with the filters.rawImport message type in messageResponser.js from adblockplusui. However, we couldn't use it before as it didn't send a response when the filter was successfully added, but it sent only a message when an error occurred. This however got fixed with #3761.
What to change
- Update the adblockplusui dependency to the latest revision including #3761
- Use filters.rawImport instead add-filters for the "Block element" dialog (block.html)
- Remove the logic to handle add-filters messages
Hints for testers
Make sure that filters added with the "Block element" dialog (availaable from the icon menu and context menu) are saved correctly. Also make sure that filter validation is applied as described here.
Attachments (0)
Change History (6)
comment:1 Changed on 03/09/2016 at 08:30:59 AM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 03/09/2016 at 06:12:28 PM by abpbot
comment:4 Changed on 03/09/2016 at 06:13:02 PM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:6 Changed on 04/20/2016 at 02:15:06 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
ABP 1.11.0.1597
Chrome 32 / 44 / 48 / Windows 8
Opera 24 / 28 / Windows 8
Chrome 44 / Ubuntu 14.04
Safari 6 / 8 / 9 / OS X 10.8 / 10.10 / 10.11
A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockpluschrome/rev/4f875bbc3244