Add prefs.set message handler

[kzar]

Background

For #5593 we're switching to using messaging for the popup window, instead of accessing the background context directly. We can call toggleIgnoreCategory() in adblockpluscore/lib/notification.js by using the prefs.toggle message, but unfortunately that doesn't allow us to pass through the optional forceValue parameter.

It was assumed that toggling notifications to be ignored / unignored was sufficient, since if a notification is displayed we know that notifications aren't being ignored and therefore toggling would always result in them being ignored. Unfortunately though, as Manish points out, this is not true since relentless notifications are displayed even if notifications are set to be ignored. (​For more context see this discussion.)

What to change

Add a prefs.set listener in messageResponders.js, which acts similarly to prefs.toggle but passes through the value parameter through to toggleIgnoreCategory if the message key is notifications_ignoredcategories and simply assigns the value to Prefs[message.key] otherwise.

[greiner]

While at it honour the forceValue option for other preferences too.

Which preferences are you referring to with that?

[kzar]

Well for all other message keys the handler does Prefs[message.key] = !Prefs[message.key];, so I figure we might as well honour the forceValue parameter for those too, so the interface is consistent. But I don't mind, if you prefer I can remove that logic too.

[greiner]

Replying to kzar:

Well for all other message keys the handler does Prefs[message.key] = !Prefs[message.key];, so I figure we might as well honour the forceValue parameter for those too, so the interface is consistent. But I don't mind, if you prefer I can remove that logic too.

Ok, makes sense to extend the logic for all preferences across the board with this parameter.

[kzar]

What do you think Thomas? I've updated the issue with Manish's idea of instead having a new message called prefs.set.

[mjethani]

If it's called prefs.set, then I would give it key and value parameters.

[kzar]

Yea I already did that, just forgot to change the name in the description.

[greiner]

Replying to kzar:

What do you think Thomas? I've updated the issue with Manish's idea of instead having a new message called prefs.set.

Should be fine as long as we're not introducing any unreasonable amount of duplication.

[sebastian]

How about using similar semantics as ​DOMTokenList.toggle(), i.e. adding an optional argument to prefs.toggle that sets the value, so that we don't need to introduce yet another message type?

[abpbot]

A commit referencing this issue has landed:
​Issue 5847 - Add prefs.set message handler

[kzar]

Replying to sebastian:

How about using similar semantics as ​DOMTokenList.toggle(), i.e. adding an optional argument to prefs.toggle that sets the value, so that we don't need to introduce yet another message type?

Well I did it that way originally but Manish preferred the message to be separate. I didn't mind too much either way, but since this is reviewed and pushed now let's just go with it.