Opened on 04/01/2019 at 11:48:05 AM

Closed on 04/16/2019 at 12:09:20 PM

Last modified on 04/19/2019 at 07:09:20 AM

#7430 closed defect (fixed)

UI not getting notified of change when toggling preference

Reported by: greiner Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-for-Chrome-Opera-Firefox-next
Module: Platform Keywords:
Cc: sebastian, kzar, Ross Blocked By:
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Unknown Verified working: yes
Review URL(s):

Description (last modified by kzar)


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 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.

Attachments (0)

Change History (14)

comment:1 Changed on 04/01/2019 at 02:21:20 PM by 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.

comment:2 Changed on 04/01/2019 at 02:24:37 PM by 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.

comment:3 Changed on 04/01/2019 at 02:32:42 PM by 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?

comment:4 Changed on 04/01/2019 at 03:22:38 PM by 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:

  •{foo: <value>})

comment:5 Changed on 04/02/2019 at 03:11:53 AM by sebastian

  • Priority changed from Unknown to P2
  • Ready set

Can we please file a Firefox bug?

We should also implement a workaround.

comment:6 Changed on 04/02/2019 at 09:23:47 AM by 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. :)

comment:7 Changed on 04/03/2019 at 03:38:09 PM by kzar

  • Description modified (diff)

Can we please file a Firefox bug?

Done that now, see

comment:9 Changed on 04/08/2019 at 03:27:16 PM by kzar

  • Owner set to kzar

comment:10 Changed on 04/08/2019 at 04:08:18 PM by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:11 Changed on 04/16/2019 at 12:00:33 PM by abpbot

comment:12 Changed on 04/16/2019 at 12:05:11 PM by kzar

  • Cc Ross added
  • Description modified (diff)
  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next

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.

comment:13 Changed on 04/16/2019 at 12:09:20 PM by kzar

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

comment:14 Changed on 04/19/2019 at 07:09:20 AM by rscott

  • Verified working set

This LGTM in ABP/FF =)

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 kzar.
Note: See TracTickets for help on using tickets.