Skip to content

Bug 2078656 - Fix firefox-prefs tools and migrate them to BiDi - #202

Merged
hbenl merged 1 commit into
mozilla:mainfrom
hbenl:bug2078656
Oct 6, 2026
Merged

hbenl merged 1 commit into
mozilla:mainfrom
hbenl:bug2078656

Conversation

@hbenl

@hbenl hbenl commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Note that this PR shares a commit with #201, so it'll have to be rebased after merging that PR.

@hbenl
hbenl requested a review from juliandescottes October 6, 2026 14:41
@hbenl hbenl added the run-integration Trigger integration tests for a PR label Oct 6, 2026
@hbenl
hbenl force-pushed the bug2078656 branch 2 times, most recently from 1933458 to 0b34199 Compare October 6, 2026 15:46

@juliandescottes juliandescottes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment about error handling, but otherwise this looks fine to me, thanks!

Comment thread src/tools/firefox-prefs.ts Outdated
Comment on lines 107 to 112

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new getPrivilegedContext helper, I think we will no longer hit this branch when MOZ_REMOTE_ALLOW_SYSTEM_ACCESS is not set.

The error will be swallowed in getPrivilegedContext and we will throw an error which doesn't preserve the original message with UnsupportedOperationError.

Maybe we can drop this try/catch? Same comment for getFirefoxPrefsTool

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

@hbenl
hbenl merged commit 5c69888 into mozilla:main Oct 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-integration Trigger integration tests for a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants