Skip to content

Conversation

@ih-codes
Copy link
Collaborator

@ih-codes ih-codes commented Jan 5, 2026

📜 Tickets

Jira ticket
Github issue

💡 Description

Workaround for the fact Reader Mode breaks once strict concurrency checking and/or Swift 6 is enabled.

We were getting a black screen when showing ReaderMode

Turns out we are getting a 401 in the JavaScript console, presumably from the GCDWebServer (in the Safari JavaScript console... see FXIOS-14565 for instructions on setting that up if you're curious). This only happens when building release builds (e.g. build FirefoxBeta).

@issammani realized we can just disable the basic auth (which isn't doing anything anyway for reader mode).

However, we should look toward actually fixing this tech debt this year (epic: FXIOS-14566). It's better to use the regular WebKit WKURLSchemeHandler and ditch GCDWebServers (old unsupported objective C library that's preconcurrency).

e.g. We can follow what Translations does:
https://github.com/mozilla-mobile/firefox-ios/blob/main/firefox-ios/Client/Frontend/Translations/SchemeHandler/TranslationsSchemeHandler.swift#L8-L40

📝 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

@ih-codes
Copy link
Collaborator Author

ih-codes commented Jan 5, 2026

I still need to test that this doesn't have any unintended side effects on the error pages which apparently use the web server as well.

Edit:

    @MainActor
    func setUpWebServer() {
        guard !webServer.server.isRunning else { return }

        readerModeHander.register(webServer, profile: profile)
        let responders: [(String, InternalSchemeResponse)] =
             [(AboutHomeHandler.path, AboutHomeHandler()),
              (AboutLicenseHandler.path, AboutLicenseHandler()),
              (ErrorPageHandler.path, ErrorPageHandler())]
        responders.forEach { (path, responder) in
            InternalSchemeHandler.responders[path] = responder
        }

Seems this is what we're registering handlers for (e.g. about home, licenses page, etc.)

@mobiletest-ci-bot
Copy link

Messages
📖 Project coverage: 38.09%

🧹 Tidy commit

Just 1 file(s) touched. Thanks for keeping it clean and review-friendly!

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

✅ Per-file coverage

All changed files meet the threshold of 35.0%.

Client.app: Coverage: 37.25

File Coverage
WebServer.swift 0.0% ⚠️

Generated by 🚫 Danger Swift against 403bceb

@issammani
Copy link
Collaborator

I still need to test that this doesn't have any unintended side effects on the error pages which apparently use the web server as well.

It shouldn't. I would expected error pages to also throw 401s if they are using the same server. AFAICT, I don't see us using auth headers anywhere in code ( and even if we are we probably shouldn't be doing it anyways there is no need for it )

@ih-codes
Copy link
Collaborator Author

ih-codes commented Jan 6, 2026

I still need to test that this doesn't have any unintended side effects on the error pages which apparently use the web server as well.

It shouldn't. I would expected error pages to also throw 401s if they are using the same server. AFAICT, I don't see us using auth headers anywhere in code ( and even if we are we probably shouldn't be doing it anyways there is no need for it )

Ahh good to know, thanks @issammani !! I'll merge this in so we can test on the next nightly.

@ih-codes ih-codes merged commit ea1094e into main Jan 6, 2026
8 checks passed
@ih-codes ih-codes deleted the ih/readermode-webserver-workaround branch January 6, 2026 15:21
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2026

🚀 PR merged to main, targeting version: 147.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants