Open
Bug 1499350
Opened 6 years ago
Updated 2 years ago
Look at removing _storingAction logic from preferences' main.js now that the feed handler code is gone
Categories
(Firefox :: Settings UI, enhancement, P3)
Firefox
Settings UI
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox64 | --- | affected |
People
(Reporter: Gijs, Unassigned)
References
Details
`_storingAction` is used as a guard while calling _storeAction to avoid "notification-triggered updates to the list while we make changes that may spawn such updates". It's clear that feeds were part of the reason this was necessary. What's less clear is if, now that feeds are gone, we can remove this complexity. After the patch from bug 1477669, we read _storeAction only in the observe() method that detects pref changes: // Rebuild the list when there are changes to preferences that influence // whether or not to show certain entries in the list. if (!this._storingAction) { // These two prefs alter the list of visible types, so we have to rebuild // that list when they change. if (aData == PREF_SHOW_PLUGINS_IN_LIST || aData == PREF_HIDE_PLUGINS_WITHOUT_EXTENSIONS) { await this._rebuildVisibleTypes(); this._sortVisibleTypes(); } // All the prefs we observe can affect what we display, so we rebuild // the view when any of them changes. this._rebuildView(); } At least within main.js, it doesn't seem like we will be changing the plugin-related preferences, and they are now the only preferences we observe here. So I *think* this code can now be removed, but I'm not sure - after all, why would the plugin observe code have been added inside the _storingAction guard code if it wasn't possible for those prefs to have been changed as a result of storing an action? The outcome of this bug could be that we remove the complexity completely, or that we keep it (though the extra `if` checking which pref is changed is redundant now and can probably be removed) and add more detailed comments about what the purpose of the _storingAction bool is.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•