Closed Bug 1966542 Opened 1 year ago Closed 9 months ago

Add support for config-based prefs that need to respond to changes in more than one pref

Categories

(Firefox :: Settings UI, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
143 Branch
Tracking Status
firefox143 --- fixed

People

(Reporter: hjones, Assigned: mkennedy)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [recomp])

Attachments

(1 file, 1 obsolete file)

There are many inputs in preferences where the pref that controls whether or not the input is disabled, hidden, etc. is different from the main pref that's controlled by the input and used to set it's value, checked state, etc. This means we'll need to introduce a mechanism for making settings respond to changes in "dependent" prefs.

The browserRestoreSession checkbox that we implemented basic disabled support for in bug 1966506 is a good example of this. It's mainly related to the browser.startup.page pref, but gets enabled or disabled depending on the value of browser.privatebrowsing.autostart. Although it should already be reading from that pref on initial render, we need to find a way to make it reactive so that the checkbox will get reenabled or disabled when the value of browser.privatebrowsing.autostart changes.

One way we could do this is by introducing a dependencies array to the setting configuration:

Preferences.addSetting({
  id: "browserRestoreSession",
  pref: "browser.startup.page",
  // add a new "dependencies" array for additional prefs we want to listen for changes on
  dependencies: ["browser.privatebrowsing.autostart"],
  disabled: () => {
    ...
  },
  ...
});

We'll then need to add logic to the Setting class to listen for changes to each pref in the dependencies array in order to emit a change event. In settings-control we may need to change this onSettingChange listener to just call this.requestUpdate to ensure that the input will re-render when one of the dependent prefs changes. We may or may not want to handle setting the value in willUpdate instead.

Acceptance criteria:

  • inputs react to changes in "dependent" i.e. prefs other than the one used to determine an input's value
  • inputs can be disabled/reenabled in reaction to changes in dependent prefs
  • add tests to cover all of the above
Blocks: 1971212
Assignee: nobody → mkennedy
  • Make Typescript work in VSCode via references, which also addresses
    performance issues (e.g. slowness of tsserver upon VSCode startup)

  • Include preferenceBindings types in mjs and js files in toolkit and browser directories

  • Move Setting and Preference classes out of Preferences IIFE so they can be
    referenced in setting-control.mjs and setting-group.mjs files.

  • Ran mach ts paths to generate missing preferencesBindings.js path resolution

  • Install and use Lit types for html return values of TemplateResult

Attachment #9496776 - Attachment description: WIP: Bug 1966542 - Get Settings and Preferences types workin r=mstriemer → Bug 1966542 - Get Settings and Preferences types workin r=mstriemer
Attachment #9496776 - Attachment description: Bug 1966542 - Get Settings and Preferences types workin r=mstriemer → WIP: Bug 1966542 - Get Settings and Preferences types workin r=mstriemer
Blocks: 1966506
No longer depends on: 1966506

Comment on attachment 9496776 [details]
WIP: Bug 1966542 - Get Settings and Preferences types workin r=mstriemer

Revision D254993 was moved to bug 1976049. Setting attachment 9496776 [details] to obsolete.

Attachment #9496776 - Attachment is obsolete: true
  • Allow Lit to update controls even when they are hidden to get visible to fire

  • Use auto scrolling checkbox as guinea pig example

Attachment #9501053 - Attachment description: WIP: Bug 1966542 - Add support for config-based prefs that need to respond to changes in more than one pref r=recomp-reviewers → Bug 1966542 - Add support for config-based prefs that need to respond to changes in more than one pref r=#recomp-reviewers,mstriemer
Pushed by mkennedy@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/67fa2b550855 https://hg.mozilla.org/integration/autoland/rev/9c3da5dde567 Add support for config-based prefs that need to respond to changes in more than one pref r=mstriemer,reusable-components-reviewers
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 143 Branch
QA Whiteboard: [qa-triage-done-c144/b143]
Regressions: 2037407
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: