Add FXIOS-11179 [Dark Mode] Use darkreader for webviews #24327

issammani · 2025-01-24T20:24:34Z

📜 Tickets

Jira ticket
Github issue

💡 Description

This PR:

Pulls in darkreader into the app.
Adds JS interface to switch between darkreader and old night mode depending on feature flag.
Adds darkreader to Licenses.html
Adds darkreader script to its own namespace isolated from other scripts.

📝 Checklist

You have to check all boxes before merging

Filled in the above information (tickets numbers and description of your work)
Updated the PR name to follow our PR naming guidelines
Wrote unit tests and/or ensured the tests suite is passing
When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
If needed, I updated documentation / comments for complex code and public methods
If needed, added a backport comment (example @Mergifyio backport release/v120)

mobiletest-ci-bot · 2025-01-24T20:36:44Z

	Messages
📖	Project coverage: 33.95%
📖	Edited 18 files
📖	Created 5 files

Client.app: Coverage: 32.3

File	Coverage
BrowserViewController.swift	3.52%	⚠️
UserScriptManager.swift	97.98%	✅
Tab.swift	42.24%	⚠️
NightModeHelper.swift	24.39%	⚠️
NimbusFlaggableFeature.swift	99.31%	✅
NimbusFeatureFlagLayer.swift	64.14%	✅

WebEngine: Coverage: 71.09

File	Coverage
WKEngineConfiguration.swift	20.83%	⚠️
WKContentScriptManager.swift	69.05%	✅

Generated by 🚫 Danger Swift against d95d709

firefox-ios/Client/Frontend/UserContent/UserScripts/MainFrame/AtDocumentEnd/DarkReader.js

...ios/Client/Frontend/UserContent/UserScripts/MainFrame/AtDocumentEnd/LegacyNightModeHelper.js

lmarceau · 2025-01-27T15:13:21Z

Nit; could you fix the tasks link in the PR description? 😄 I wanted to have a look at them since I was curious about the work!

issammani · 2025-01-27T15:23:58Z

Nit; could you fix the tasks link in the PR description? 😄 I wanted to have a look at them since I was curious about the work!

Sure. I was too lazy to do it last Friday 😂 Done now. I added a link to the spec doc in the jira ticket.

nbhasin2 · 2025-01-29T19:40:06Z

BrowserKit/Sources/WebEngine/WKWebview/Scripts/WKContentScriptManager.swift

+        // If a script already exists on the page, skip adding this duplicate.
+        guard scripts[name] == nil else { return }


dataports · 2025-01-30T00:23:09Z

This looks great! Is this a candidate to add to the feature flags toggle panel in the secret settings menu for testing purposes?

I noticed while testing that there was this weirdness with Twitter/X Google button but I'm guessing that's just them having a bad asset? (This is with Night Mode enabled)

I don't really feel qualified to hit approve on this because I'm not a JS dev at alllll, so I'll let @nbhasin2 do it, but the swift looks good to me! TIL what WKContentWorld is ⭐

nbhasin2 · 2025-01-30T00:40:21Z

That google button is odd but I doubt we can do anything with it. My understanding is it can be fixed by DarkReader when they update their scripts.

issammani · 2025-01-30T02:21:39Z

I noticed while testing that there was this weirdness with Twitter/X Google button but I'm guessing that's just them having a bad asset? (This is with Night Mode enabled)

Ok I prematurely bumped the package version yesterday 🙈 . I downgraded it now and it behaves as expected for x.com. I tried the build from source and it also behaves correctly. It seems to be a bad version in between. Will update to the latest version once that's published to npm. Thanks for flagging @dataports 😄

My understanding is it can be fixed by DarkReader when they update their scripts.

Exactly, this is the goal. They merge a few PRs a day fixing things so we should benefit from those fixes. Once we finish up phase 3, we should be updating this more regularly.

…eatment

Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com>

issammani · 2025-01-30T02:50:57Z

Ok BR is failing. Will check back tomorrow what's happening. Seems unrelated...

issammani · 2025-01-30T15:39:04Z

@Mergifyio backport release/v135

mergify · 2025-01-30T15:39:16Z

backport release/v135

✅ Backports have been created

#24453 Add FXIOS-11179 [Dark Mode] Use darkreader for webviews (backport #24327) has been created for branch release/v135 but encountered conflicts

* chore: add darkreader lib * feat: add nimbus flag * feat: pass down flag to JS * feat: move old code to LegacyNightModeHelper and add logic to pick treatment * fix: lint * fix: wrong flag value * fix: cleanup flag check * fix: typo Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> * fix: remove vim modeline Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> * chore: add license * chore: move night mode to separate dir * feat: isolate nightmode into its own content world * fix: update mocks * fix: downgrade darkreader to previous version --------- Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> (cherry picked from commit 3fbead0) # Conflicts: # firefox-ios/Client/FeatureFlags/NimbusFlaggableFeature.swift

) (#24453) * Add FXIOS-11179 [Dark Mode] Use darkreader for webviews (#24327) * chore: add darkreader lib * feat: add nimbus flag * feat: pass down flag to JS * feat: move old code to LegacyNightModeHelper and add logic to pick treatment * fix: lint * fix: wrong flag value * fix: cleanup flag check * fix: typo Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> * fix: remove vim modeline Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> * chore: add license * chore: move night mode to separate dir * feat: isolate nightmode into its own content world * fix: update mocks * fix: downgrade darkreader to previous version --------- Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com> (cherry picked from commit 3fbead0) # Conflicts: # firefox-ios/Client/FeatureFlags/NimbusFlaggableFeature.swift * fix: merge conflict --------- Co-authored-by: Issam Mani <issamouu69@gmail.com>

nbhasin2 reviewed Jan 24, 2025

View reviewed changes

firefox-ios/Client/Frontend/UserContent/UserScripts/MainFrame/AtDocumentEnd/DarkReader.js Outdated Show resolved Hide resolved

nbhasin2 reviewed Jan 24, 2025

View reviewed changes

...ios/Client/Frontend/UserContent/UserScripts/MainFrame/AtDocumentEnd/LegacyNightModeHelper.js Outdated Show resolved Hide resolved

issammani changed the title ~~Add [WIP] [Dark Mode] Use darkreader for webviews~~ Add FXIOS-11179 [Dark Mode] Use darkreader for webviews Jan 27, 2025

issammani marked this pull request as ready for review January 29, 2025 15:20

issammani requested a review from a team as a code owner January 29, 2025 15:20

issammani requested a review from dataports January 29, 2025 15:20

issammani force-pushed the poc/dark-reader-lib branch 2 times, most recently from 52a7f00 to d95d709 Compare January 29, 2025 16:39

nbhasin2 reviewed Jan 29, 2025

View reviewed changes

nbhasin2 approved these changes Jan 30, 2025

View reviewed changes

issammani and others added 14 commits January 30, 2025 03:37

chore: add darkreader lib

d9dee10

feat: add nimbus flag

45d5b11

feat: pass down flag to JS

ad86aa2

feat: move old code to LegacyNightModeHelper and add logic to pick tr…

178dbcc

…eatment

fix: lint

3031f05

fix: wrong flag value

10c956e

fix: cleanup flag check

a6af0bd

fix: typo

3a4c2c6

Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com>

fix: remove vim modeline

76fc8cd

Co-authored-by: Nishant Bhasin <nish.bhasin@gmail.com>

chore: add license

51a3439

chore: move night mode to separate dir

a68be28

feat: isolate nightmode into its own content world

bf07712

fix: update mocks

d24c6d2

fix: downgrade darkreader to previous version

056dccf

issammani force-pushed the poc/dark-reader-lib branch from 96902ad to 056dccf Compare January 30, 2025 02:37

issammani merged commit 3fbead0 into main Jan 30, 2025
13 checks passed

issammani deleted the poc/dark-reader-lib branch January 30, 2025 15:38

mergify bot mentioned this pull request Jan 30, 2025

Add FXIOS-11179 [Dark Mode] Use darkreader for webviews (backport #24327) #24453

Merged

6 tasks

issammani mentioned this pull request Feb 10, 2025

[bug] Scrollbar invisible on FF iOS when using “enable night mode” (and dark theme on iOS) #7639

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FXIOS-11179 [Dark Mode] Use darkreader for webviews #24327

Add FXIOS-11179 [Dark Mode] Use darkreader for webviews #24327

issammani commented Jan 24, 2025 •

edited

Loading

mobiletest-ci-bot commented Jan 24, 2025 •

edited

Loading

lmarceau commented Jan 27, 2025

issammani commented Jan 27, 2025

nbhasin2 Jan 29, 2025

dataports commented Jan 30, 2025

nbhasin2 commented Jan 30, 2025

issammani commented Jan 30, 2025 •

edited

Loading

issammani commented Jan 30, 2025

issammani commented Jan 30, 2025

mergify bot commented Jan 30, 2025 •

edited

Loading

		// If a script already exists on the page, skip adding this duplicate.
		guard scripts[name] == nil else { return }

Add FXIOS-11179 [Dark Mode] Use darkreader for webviews #24327

Add FXIOS-11179 [Dark Mode] Use darkreader for webviews #24327

Conversation

issammani commented Jan 24, 2025 • edited Loading

📜 Tickets

💡 Description

📝 Checklist

mobiletest-ci-bot commented Jan 24, 2025 • edited Loading

Client.app: Coverage: 32.3

WebEngine: Coverage: 71.09

lmarceau commented Jan 27, 2025

issammani commented Jan 27, 2025

nbhasin2 Jan 29, 2025

Choose a reason for hiding this comment

dataports commented Jan 30, 2025

nbhasin2 commented Jan 30, 2025

issammani commented Jan 30, 2025 • edited Loading

issammani commented Jan 30, 2025

issammani commented Jan 30, 2025

mergify bot commented Jan 30, 2025 • edited Loading

✅ Backports have been created

issammani commented Jan 24, 2025 •

edited

Loading

mobiletest-ci-bot commented Jan 24, 2025 •

edited

Loading

issammani commented Jan 30, 2025 •

edited

Loading

mergify bot commented Jan 30, 2025 •

edited

Loading