Refactor Bug 2003236 refactor NimbusClient params by tiftran · Pull Request #31678 · mozilla-mobile/firefox-ios

tiftran · 2026-01-16T05:29:13Z

📜 Tickets

https://bugzilla.mozilla.org/show_bug.cgi?id=2003236

💡 Description

Hello IOS friends!
There was some refactor work in app service PR here
I tried to make the corresponding changes to IOS.
Essentially we're passing along NimbusServerSettings as a param along with contains RemoteSettingsService and collection_name rather passing the two separately.

🎥 Demos

Before	After

Demo

📝 Checklist

I filled in the ticket numbers and a description of my work
I updated the PR name to follow our PR naming guidelines
I ensured unit tests pass and wrote tests for new code
If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
If adding telemetry, I read the data stewardship requirements and will request a data review
If adding or modifying strings, I read the guidelines and will request a string review from l10n
If needed, I updated documentation and added comments to complex code

tiftran · 2026-01-16T05:37:33Z

focus-ios/Blockzilla/Nimbus/NimbusExtensions.swift

+        return Bundle.main.object(forInfoDictionaryKey: key) as? String
    }
+
+    static func remoteSettingsAppContext() -> RemoteSettingsContext {


I mostly copied this over from ios. I'm not sure if this is the best way to go about it

tiftran · 2026-01-16T05:38:05Z

focus-ios/Blockzilla/Nimbus/NimbusExtensions.swift

+            case .mac: "desktop"
+            default: "phone"
+            }
+            let regionCode = Locale.current.regionCode


this was changed though, since Focus doesn't have storage, though im not sure if this is correct

tiftran · 2026-01-16T05:38:18Z

Hello IOS friends!
I don't have requesting review powers so tagging @issammani @mattreaganmozilla since youve looked at my nimbus related changes in the past 🙂
and @freshstrangemusic to ensure I haven't messed anything up for nimbus 🙂

I'm not sure what the Focus:reference string linter is referring to... the logs doesn't seem to point to anything to me

PARAIPAN9 · 2026-01-16T08:54:29Z

Tagging @adudenamedruby. He's the Nimbus Wizard!! 🧙

adudenamedruby · 2026-01-19T14:23:53Z

@mergify rebase

mergify · 2026-01-19T14:24:14Z

rebase

✅ Branch has been successfully rebased

adudenamedruby · 2026-01-19T14:46:21Z

@tiftran Heya. Seems both focus strings script and bitrise are failing. Oh noes!

re: strings: can you make sure your branch is rebased on the latest main and then, locally, do a sh focus-ios/focus-ios-tests/tools/export-strings.sh and see if it fails locally and what the export-log.txt has if it fails?

re: bitrise: is this building successfully locally? Both firefox and focus fail to build with the same error:

...MozillaRustComponents/Sources/FocusRustComponentsWrapper/Generated/nimbus.swift:764:80: error: missing argument for parameter #9 in call
...MozillaRustComponents/Sources/FocusRustComponentsWrapper/Generated/nimbus.swift:764:78: error: cannot convert value of type 'UnsafeMutablePointer<RustCallStatus>' to expected argument type 'RustBuffer' (exit code: 1)

tiftran · 2026-01-19T20:39:46Z

Hi @adudenamedruby,
rebased and I got

[*] Cloning mozilla-l10n/focusios-l10n
Cloning into 'focusios-l10n'...
remote: Enumerating objects: 29399, done.
remote: Counting objects: 100% (174/174), done.
remote: Compressing objects: 100% (59/59), done.
remote: Total 29399 (delta 124), reused 138 (delta 92), pack-reused 29225 (from 2)
Receiving objects: 100% (29399/29399), 25.06 MiB | 4.48 MiB/s, done.
Resolving deltas: 100% (16491/16491), done.
[*] Cloning mozilla-mobile/LocalizationTools
Cloning into 'focus-ios-tests/tools/Localizations'...
remote: Enumerating objects: 177, done.
remote: Counting objects: 100% (177/177), done.
remote: Compressing objects: 100% (104/104), done.
remote: Total 177 (delta 73), reused 134 (delta 44), pack-reused 0 (from 0)
Receiving objects: 100% (177/177), 45.80 KiB | 1.48 MiB/s, done.
Resolving deltas: 100% (73/73), done.
[*] Building tools/Localizations
Fetching https://github.com/apple/swift-argument-parser
Fetched https://github.com/apple/swift-argument-parser from cache (3.53s)
Computing version for https://github.com/apple/swift-argument-parser
Computed https://github.com/apple/swift-argument-parser at 0.4.4 (3.93s)
Creating working copy for https://github.com/apple/swift-argument-parser
Working copy of https://github.com/apple/swift-argument-parser resolved at 0.4.4
Building for debugging...
[49/49] Applying LocalizationTools
Build complete! (9.10s)
[*] Replacing firefox with focus in swift task files
[*] Updating EXPORT_BASE_PATH with (getpid()) in swift export task
[*] Exporting Strings (output in export-strings.log)

makes me think im missing something...

as for bitrise, it runs locally for me when I have it set up to use my local app services with the related changes.
Bitrise should be failing right? considering the related app services changes haven't merged yet?

mattreaganmozilla · 2026-01-20T18:23:27Z

focus-ios/Blockzilla/Nimbus/NimbusExtensions.swift

+            return RemoteSettingsContext(
+                channel: appInfo.buildChannel?.rawValue ?? "release",
+                appVersion: AppInfo.appVersion,
+                appId: AppInfo.bundleIdentifier,
+                /// `Locale.current.identifier` uses an underscore (e.g. “en_US”), which is not supported by RS.
+                /// Nimbus’s `getLocaleTag()` returns a Gecko-compatible locale (e.g. “en-US”).
+                /// In Gecko, we use BCP47 format, specifically `appLocaleAsBCP47`
+                /// See : https://searchfox.org/mozilla-central/rev/240ca3f/toolkit/modules/RustSharedRemoteSettingsService.sys.mjs#46
+                /// Once we drop support for iOS <16 we can support the proper  BCP47 by using `Locale.IdentifierType.bcp47`
+                /// See: https://developer.apple.com/documentation/foundation/locale/identifiertype/bcp47
+                locale: getLocaleTag(),
+                os: "iOS",
+                osVersion: UIDeviceDetails.systemVersion,
+                formFactor: formFactor,
+                country: country)
+        }


Is this basically bringing Remote Settings over to Focus? 🤔 Focus is in maintenance mode, and it seems like if we want to integrate Remote Settings into it properly that's going to be a whole project unto itself.

For example using Remote Settings in any client (Firefox or Focus) also requires code for syncing the collections etc. (In Firefox currently that happens in RemoteSettingsServiceSyncCoordinator).

My understanding based on past iOS team conversations is that we had not planned to update Focus to use Remote Settings.

@mattreaganmozilla ooh if Remote Settings Service shouldn't be in focus, then this is definitely not what I actually want to do.
My app services PR tries to change the Nimbus Client params to use NimbusServerSettings instead of passing RemoteSettingsService and CollectionName around everywhere. So I tried to repurpose the existing NimbusServerSettings for that. Maybe not worth it, if this is causing complexity to Focus.

Though, looking at it again (im a total ios dummy, so i probs 100% wrong), Is NimbusServerSettings.createFromInfoDictionary used anywhere? I see NimbusAppSettings.createFromInfoDictionary being used but not the former. I also tried renaming it to see if the build would break too.

@tiftran I could be misunderstanding, so apologies for any confusion. My concern was that using RemoteSettingsService directly in Focus would add additional complexity beyond just the requirements for this PR, but I don't have full context on this update. Maybe we don't need anything special besides this (no support for calling into service.sync()?)

@issammani is our resident AS+iOS wizard so I'm curious if he has more context or thoughts.

@mattreaganmozilla I spoke with @freshstrangemusic, and since there will be an eventual removal of Nimbus related code, she advised to pass RemoteSettingsService as nil for now, so therefore all this extra remote settings logic i tried to shove in to make things work, would go away. Would like your perspective before going ahead and doing so.

so therefore all this extra remote settings logic i tried to shove in to make things work, would go away

Ok gotcha, SGTM. 👍 Thanks @tiftran

Changes made, I think it looks better? Though, Im still getting the Focus: reference string linter / build (26.0) check failing. Could I get some assistance please @mattreaganmozilla or @adudenamedruby? 🥺

I'm not sure where github stores the artifacts (that export-strings.log file). Can you check locally what the error is or what's in that log?? Also, make sure you're rebased to the latest main - I made some changes for l10n - and looking at the GH action log, I think you've already ingested them, but just good to double check. If we don't figure it out from the log, set up a quick meeting with me (Tuesday or after, please as I'll be PTO Monday) and we'll try and debug.

Also, what's the merge order for these changes? The AS needs to be merged first, right? Otherwise bitrise won't pass and we can't merge this.

@tiftran Can you DM me (or @adudenamedruby) in Slack if you end up becoming blocked on this? I didn't want this PR to slip under the radar but I also am not clear on what the remaining issues are (if it's just the linter & strings issues then Roux will know much more than me, but if I can help with anything else LMK). I'm going to hold off on re-reviewing in the meantime.

mergify · 2026-01-29T22:00:14Z

This pull request has conflicts when rebasing. Could you fix it @tiftran? 🙏

thatswinnie · 2026-02-04T14:07:53Z

@tiftran I was able to build pointing to your AS branch locally. And with the build being successful the linter passes as well now locally. This branch has conflicts now however so could you update it against main please? After that @mattreaganmozilla and @adudenamedruby could review the PR.

tiftran · 2026-02-04T19:03:22Z

rebase completed

tiftran requested a review from a team as a code owner January 16, 2026 05:29

tiftran requested a review from PARAIPAN9 January 16, 2026 05:29

tiftran commented Jan 16, 2026

View reviewed changes

PARAIPAN9 requested review from adudenamedruby and removed request for PARAIPAN9 January 16, 2026 08:54

adudenamedruby force-pushed the nimbus-ios-follow-up branch from 13a19ff to a6fb034 Compare January 19, 2026 14:24

mattreaganmozilla reviewed Jan 20, 2026

View reviewed changes

tiftran force-pushed the nimbus-ios-follow-up branch 3 times, most recently from 08253a3 to c02a267 Compare January 23, 2026 05:09

FilippoZazzeroni requested a review from mattreaganmozilla January 27, 2026 08:42

tiftran force-pushed the nimbus-ios-follow-up branch from c02a267 to e266f8b Compare January 28, 2026 15:55

Refactor Bug 2003236 refactor NimbusClient params

2d4d47c

tiftran force-pushed the nimbus-ios-follow-up branch from e266f8b to 2d4d47c Compare February 4, 2026 18:50

Conversation

tiftran commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Tickets

💡 Description

🎥 Demos

📝 Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiftran commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PARAIPAN9 commented Jan 16, 2026

Uh oh!

adudenamedruby commented Jan 19, 2026

Uh oh!

mergify bot commented Jan 19, 2026

✅ Branch has been successfully rebased

Uh oh!

adudenamedruby commented Jan 19, 2026

Uh oh!

tiftran commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiftran Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jan 29, 2026

Uh oh!

thatswinnie commented Feb 4, 2026

Uh oh!

tiftran commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tiftran commented Jan 16, 2026 •

edited

Loading

tiftran commented Jan 16, 2026 •

edited

Loading

tiftran Jan 21, 2026 •

edited

Loading