Opened on 02/12/2019 at 01:14:11 PM
Closed on 02/12/2019 at 02:26:55 PM
Last modified on 02/14/2019 at 10:22:53 AM
#7278 closed defect (fixed)
Whitelist entries cannot be added from the Settings page
Reported by: | Ross | Assignee: | greiner |
---|---|---|---|
Priority: | P1 | Milestone: | |
Module: | User-Interface | Keywords: | |
Cc: | sebastian, kzar, geo, greiner | Blocked By: | |
Blocking: | #7279 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | yes |
Review URL(s): |
https://gitlab.com/eyeo/adblockplus/abpui/adblockplusui/merge_requests/220 |
Description (last modified by greiner)
Environment
ABP 3.4.3.2252
Chrome 72.0.3626.96 / Windows 10
Firefox 65.0 / Windows 10
Edge 44.17763.1.0 / Windows 10
How to reproduce
- Select [ABP Icon] > [Cog].
- Select [Whitelisted websites]
- Enter a website/domain.
Observed behaviour
No entry is added to the whitelist.
Whitelisting from the ABP Popup works as expected.
Expected behaviour
The user to be able to add a website/domain to the whitelist from the settings page.
Notes
It seems like FilterStorage got renamed to filterStorage, but not all instances of the name change were made in adblockplusui/messageResponder.js. In this instance, the listener for the "filters.add" message attempts to call FilterStorage.addFilter.
Hints for testers
In Whitelisted websites tab in options page:
- Adding a domain to the list should work
- Removing a domain from the list should work
Attachments (0)
Change History (9)
comment:1 Changed on 02/12/2019 at 01:21:48 PM by kzar
- Cc greiner added
comment:2 Changed on 02/12/2019 at 01:31:08 PM by kzar
- Component changed from Unknown to User-Interface
- Description modified (diff)
Yep, got to the bottom of it. Any chance you could triage this one Thomas?
comment:3 Changed on 02/12/2019 at 01:52:04 PM by greiner
- Priority changed from Unknown to P1
- Ready set
I can confirm that there are some instances in the message responder where FilterStorage is still being used.
While #7151 was correctly applied, we had to back it out to be able to land some UI fixes in master. After all those fixes have landed, we then reapplied the compatibility changes. At that point, however, some of those changes, including but maybe not limited to this one, appear to have gotten lost.
comment:4 Changed on 02/12/2019 at 01:55:01 PM by greiner
- Owner set to greiner
comment:5 Changed on 02/12/2019 at 02:06:31 PM by greiner
I've looked through the diff and, apart from the various Edge fixes, the only changes that appear to have not been reapplied were related to adding and removing a single filter.
comment:6 Changed on 02/12/2019 at 02:10:21 PM by greiner
- Blocking 7279 added
comment:7 Changed on 02/12/2019 at 02:26:55 PM by greiner
- Resolution set to fixed
- Status changed from reviewing to closed
I've also created #7279 for the dependency update but note that first this change has to make it through Feature QA.
comment:8 Changed on 02/12/2019 at 05:38:22 PM by abpbot
A commit referencing this issue has landed:
Issue 7278 - Fixed: FilterStorage name change was only partially reapplied
comment:9 Changed on 02/14/2019 at 10:22:53 AM by ukacar
- Verified working set
Sounds like this might come under the User-Interface module, but investigating.