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)

enhancement

Tracking

()

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.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.