browser.storage.onChanged doesn't fire when false values are removed

RESOLVED FIXED in Firefox 67

Status

defect
P1
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: kzar, Assigned: rpl)

Tracking

(Blocks 1 bug)

66 Branch
mozilla68
Dependency tree / graph
Bug Flags:
qe-verify ?

Firefox Tracking Flags

(firefox67 fixed, firefox68 verified)

Details

(Whiteboard: [qa-triaged])

Attachments

(2 attachments)

Reporter

Description

3 months ago

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.119 Safari/537.36

Steps to reproduce:

  1. Install attached extension, opened the background console for it.

Actual results:

The following was logged:

onChanged example: { oldValue: undefined, newValue: false } local

Expected results:

The following should be logged:

onChanged example: { oldValue: undefined, newValue: false } local
onChanged example: { oldValue: false } local

Reporter

Updated

3 months ago
Component: Untriaged → Extension Compatibility
Reporter

Comment 1

3 months ago

The event works as expected on both older versions of Firefox and Chrome:

Firefox 64.0 (64-bit)

onChanged example: { oldValue: undefined, newValue: false } local
onChanged example: { oldValue: false } local

Chrome 72.0.3626.119

onChanged example: { newValue: false } local
onChanged example: { oldValue: false } local

See also the related discussion on the Adblock Plus issue tracker: https://issues.adblockplus.org/ticket/7430

Updated

3 months ago
Component: Extension Compatibility → Storage
Product: Firefox → WebExtensions
Reporter

Comment 2

3 months ago

(Should I mark issues like this as blocking #467520 or #1226547?)

Assignee

Updated

3 months ago
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Priority: -- → P1

Comment 4

3 months ago
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/176a8c9d1b7b
storage.local API should fire onChanged event when falsey values are removed. r=mixedpuppy

Comment 5

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee

Comment 6

3 months ago

Let's QA verify the fix as applied in Nightly using the STR from comment 0.

Flags: qe-verify?
Assignee

Comment 7

3 months ago

Comment on attachment 9055516 [details]
Bug 1541449 - storage.local API should fire onChanged event when falsey values are removed. r?mixedpuppy!

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1488825
  • User impact if declined: Some extension may not behave as expected because the expected onChanged event is not being fired when the changed item has a falsey value (e.g. as mentioned in comment 1, AdBlock Plus doesn't update part of its UI because the expected onChanged event has not been fired).
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: QA verification of the fix can be done using the STR provided by the reporter in comment 0.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change required to fix the issue is small and restricted to the internal function that implements the underlying behavior for the storage.local.remove API method.
    The change also includes additional test cases to ensure that this scenario is explicitly tested as part of the automated tests.
  • String changes made/needed:
Attachment #9055516 - Flags: approval-mozilla-beta?

I'll take it in Beta once QA has verified it in Nightly, thanks for the STR!

Flags: qe-verify? → qe-verify+
Whiteboard: [qa-triaged]

I’ve reproduce this issue following the steps from comment 0 with Fx 68.0a1(2019-04-03) on Ubuntu 18.04 x64.
The issue is verified fixed with Fx 68.0a1 (2019-04-10) on Ubuntu 18.04 x64, macOS 10.13 and Windows 10 x64.

Comment on attachment 9055516 [details]
Bug 1541449 - storage.local API should fire onChanged event when falsey values are removed. r?mixedpuppy!

Impacts some high volume extensions, has tests and was veirfied by QA po Nightly, uplift approved for 67 beta 11, thanks!

Attachment #9055516 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee

Comment 12

2 months ago

Comment on attachment 9055516 [details]
Bug 1541449 - storage.local API should fire onChanged event when falsey values are removed. r?mixedpuppy!

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1488825
  • User impact if declined: Same as comment 7
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: QA verification of the fix can be done using the STR provided by the reporter in comment 0.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Same as comment 7
  • String changes made/needed:
Attachment #9055516 - Flags: approval-mozilla-release?
Assignee

Updated

2 months ago
Flags: qe-verify+ → qe-verify?

So far, we don't have another dot release planned, but I'll keep this open for another week in case something else crops up.

Summary: browser.storage.onChanged doesn't fire when falsey values are removed → browser.storage.onChanged doesn't fire when false values are removed

Comment on attachment 9055516 [details]
Bug 1541449 - storage.local API should fire onChanged event when falsey values are removed. r?mixedpuppy!

We're a week or so out from 67 release. Wontfix for 66.

Attachment #9055516 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.