UI not getting notified of change when toggling preference

[greiner]

Environment

Ubuntu 16.04 / Windows
Firefox 66
Adblock Plus 3.5

How to reproduce

1. Open Advanced tab in options page
2. Click on "Show 'Adblock Plus' panel in developer tools" checkbox
3. Click on checkbox again

Observed behaviour

After step 2) Checkbox is unchecked and preference is set to false.
After step 3) Checkbox remains unchecked and preference is removed.

Expected behaviour

After step 2) Checkbox is unchecked and preference is set to false.
After step 3) Checkbox is checked and preference is removed.

Further information

Upon further investigation it seems that browser.storage.onChanged no longer notifies us when removing a preference whose value is false.

See this ​Firefox bug report.

See also ​ui#384.

Hints for testers

• The underlying Firefox bug has now been fixed and that fix included with Firefox 67. Test that our workaround here works for Firefox 66. Please also test that the options page and other UI relating to user preferences works OK in different versions of Firefox, also on Chrome.

[kzar]

I've just read through this and the linked GitLab issue, just to confirm you can't reproduce the problem in earlier versions of Firefox, nor for other platforms? If not, I wonder if this is a regression in Firefox itself.

[greiner]

According to Ursa "older versions work as expected" so it could very well be a regression by Firefox. I don't know about other browsers.

[kzar]

Just thinking out loud, but perhaps a workaround could be to store a different "falsey" value instead of false? For example, does the event fire for 0?

[greiner]

Here's the data I gathered so far:

Value	Value (initial)	onChanged	Value (set)	onChanged	Value (removed)
"foo"	undefined	undefined -> "foo"	"foo"	"foo" -> undefined	undefined
""	undefined	undefined -> ""	""	(missing)	undefined
true	undefined	undefined -> true	true	true -> undefined	undefined
false	undefined	undefined -> false	false	(missing)	undefined
1	undefined	undefined -> 1	1	1 -> undefined	undefined
0	undefined	undefined -> 0	0	(missing)	undefined
null	undefined	undefined -> null	null	(missing)	undefined
undefined	undefined	undefined -> undefined	undefined	(missing)	undefined

Therefore it seems that this issue applies to all falsey values.

PS: I gathered the data using a simple extension that runs the following for any of the given values and logs out the various data while doing so:

• browser.storage.local.set({foo: <value>})
• browser.storage.local.remove("foo")

[sebastian]

Can we please file a Firefox bug?

We should also implement a workaround.

[ukacar]

Just to confirm that this issue only happens in Firefox 66 (and 67, which is the current Beta version). Chrome, Opera and older versions of Firefox work as expected. :)

[kzar]

Can we please file a Firefox bug?

Done that now, see ​https://bugzilla.mozilla.org/show_bug.cgi?id=1541449

[kzar]

​Looks like they've fixed the Firefox bug now and its aimed for Firefox 68.

[abpbot]

Some commits referencing this issue have landed:

• ​Issue 7430 - Workaround missing storage.local.onChanged events
• ​Issue 7430 - Workaround missing storage.local.onChanged events

[kzar]

Note: This milestone is wrong, since this workaround is going to be included in the upcoming hotfix release. Once the correct milestone exists, we can update it here.

[rscott]

This LGTM in ABP/FF 3.5.1.2305. =)