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)


With #3826 Prefs.onChanged is going to be replaced by Prefs.on(pref, cb) and, 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/ 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:1 Changed on 03/18/2016 at 03:22:54 PM by sebastian

  • Description modified (diff)

comment:2 Changed on 03/18/2016 at 03:38:14 PM by greiner

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()/

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:4 Changed on 03/18/2016 at 06:46:19 PM by greiner

  • Description modified (diff)
  • Ready set

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:

comment:8 Changed on 03/22/2016 at 03:39:09 PM by sebastian

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sebastian.
Note: See TracTickets for help on using tickets.