storage.onChanged clarrification #18326
Conversation
Preview URLsFlawsNone! 🎉 External URLsURL: No new external URLs (this comment was updated 2022-08-10 17:18:13.710708) |
| @@ -15,7 +15,9 @@ browser-compat: webextensions.api.storage.onChanged | |||
| --- | |||
| {{AddonSidebar()}} | |||
|
|
|||
| Fired when one or more items change. | |||
| Fired when {{WebExtAPIRef('storage.StorageArea.set','storageArea.set')}} executes against a storage area. | |||
There was a problem hiding this comment.
@rebloor technically the onChanged event would also be fired in response to calls to the remove and clear methods, and so we should mention them here along with set.
I also confirmed that the clarification paragraph that follows (related to the behavior to expect when set is called) should be ok as is:
- storage.local: looking to the implementation for storage.local.set/remove/clear from ExtensionStorageIDB.jsm seems to confirm (as I recalled) that
removeandclearwould only fire theonChangedevent if a key was actually removed. - storage.sync: storage.sync.set should be presenting the same behavior that storage.local.set presents and as already described in the clarification paragraph as is (while
removeandclearare only fired when there were keys removed, as described above for the storage.local implementation).
There was a problem hiding this comment.
@rpl can I clarify, do remove and clear triggers also result in all the keys being returned or just the keys that were removed or cleared?
There was a problem hiding this comment.
@rpl can I clarify, do remove and clear triggers also result in all the keys being returned or just the keys that were removed or cleared?
They both fire an event only if something has been removed (or cleared), from the perspective of what is included in the changes emitted in an onChanged event:
- on
removecalls: only the keys removed are expected to be included - on
clearcalls: we will remove everything and so the changes emitted will technically include all stored keys (if there wasn't any then onChanged wouldn't be fired).
| Fired when one or more items change. | ||
| Fired when {{WebExtAPIRef('storage.StorageArea.set','storageArea.set')}} executes against a storage area. | ||
|
|
||
| As this event is triggered by {{WebExtAPIRef('storage.StorageArea.set','storageArea.set')}}, it's possible to receive a callback when there is no change to the underlying data. Also, the information returned includes all keys within the storage area {{WebExtAPIRef('storage.StorageArea.set','storageArea.set')}} ran against. Your code needs to determine what changes have occurred by examining the content of the `changes` argument. |
There was a problem hiding this comment.
Nit, I'm wondering if we could reword a bit "Your code needs to determine ..." to say something like The extension may determine what changes have occurred by examining the content of the changesargument received by theonChanged listeners.. wdyt?
|
|
||
| - `changes` | ||
| - : `object`. Object describing the change. This contains one property for each key that changed. The name of the property is the name of the key that changed, and its value is a {{WebExtAPIRef('storage.StorageChange')}} object describing the change to that item. | ||
| - : `object`. Object describing the change. This object contains properties for all the keys in the storage area where {{WebExtAPIRef('storage.StorageArea.set','storageAred.set')}} occurred. The name of each property is the name of each key. The value of each key is a {{WebExtAPIRef('storage.StorageChange')}} object describing the change to that item. |
There was a problem hiding this comment.
I just noticed that this is currently saying "all the keys in the storage area where set occurred", but the actual behavior is that the changes object will contains all keys that where included in the set call, even if the old and new value may still be the same.
Any other key stored in the same area that was not part of what was passed to the set call will still not be part of the changes object.
would you mind to tweak that part?
Summary
Clarifies the behavior of and data returned by storage.onChanged. Provides the content requested in Bug 1621162.
Metadata
This PR…