Opened on 03/18/2016 at 03:22:04 PM
Closed on 03/22/2016 at 03:39:09 PM
#3839 closed change (fixed)
Adapt for Prefs event changes
| Reported by: | sebastian | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | User-Interface | Keywords: | |
| Cc: | greiner, saroyanm | Blocked By: | |
| Blocking: | #3826 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by greiner)
Background
With #3826 Prefs.onChanged is going to be replaced by Prefs.on(pref, cb) and Prefs.off(pref, cb). So instead of having a single listener for all preference changes, you have to register one callback for each preference.
What to change
- Adapt the code for the new options page to use Prefs.on/Prefs.off instead Prefs.onChanged.
- There's no need for aggregating listeners anymore as with Prefs.onChanged so each "prefs.listen" message should be independent of each other.
Attachments (0)
Change History (8)
comment:2 Changed on 03/18/2016 at 03:38:14 PM by greiner
comment:3 Changed on 03/18/2016 at 06:37:50 PM by sebastian
Not that opening the options page is a hotspot, neither is having multiple options pages open a particular common use case (in fact we prevent that to some degree). So if I understand correctly, it doesn't matter much either way. Hence I'd prefer whatever results into a simpler implementation.
comment:5 Changed on 03/19/2016 at 07:13:03 PM by sebastian
- Blocking 3826 added
- Owner set to sebastian
comment:6 Changed on 03/19/2016 at 07:14:22 PM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:7 Changed on 03/22/2016 at 03:38:10 PM by abpbot
A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockplusui/rev/49b08709f3d2
comment:8 Changed on 03/22/2016 at 03:39:09 PM by sebastian
- Resolution set to fixed
- Status changed from reviewing to closed
Note: See
TracTickets for help on using
tickets.
I wonder whether we should have one listener per preference for all the UI or one listener per UI listener. The former would require some aggregation logic (similar to how we currently have it in messageResponder.js) while the latter may result in more calls to Prefs.on()/Prefs.off().