Configure script based on remote config #7036
Merged
+115 −1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch from a52dd72 to d84dffc Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch 2 times, most recently from 301952b to ad6c768 Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/create-js-script branch from ac2f5ab to a48fefe Compare 
CDRussell reviewed Oct 30, 2025
| private val postInitialPing = "\$POST_INITIAL_PING$" | ||
| private val replyToNativeMessages = "\$REPLY_TO_NATIVE_MESSAGES$" | ||
| | ||
| private fun configureWebViewForWebViewCompatTest(webView: DuckDuckGoWebView) { |
There was a problem hiding this comment.
can we move all of this to its own class to keep BTF changes minimal and contained? thinking there would be a single line in BTF which calls through to your thing passing the WebView for instance.
There was a problem hiding this comment.
Sure. I wanted to avoid overengineering for this project, as we did for the previous one, but happy to reach a middle ground between overengineering and purely hacking our way around it
CDRussell approved these changes Oct 30, 2025
CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch from ad6c768 to 4939603 Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/create-js-script branch from a48fefe to 1078f2d Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch from 4939603 to 9acf98b Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/create-js-script branch from 5a08c8b to e5f0620 Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch 4 times, most recently from 2c94455 to 25ece01 Compare CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/create-js-script branch from e5f0620 to 31f03cf Compare CDRussell reviewed Nov 4, 2025
Comment on lines 50 to 51
| withContext(dispatchers.main()) { | ||
| val script = withContext(dispatchers.io()) { |
There was a problem hiding this comment.
nit: i don't think this top level withContext(dispatchers.main()) helps with readability since you immediately jump to another dispatchers in the first of the nested block.
might read better to have val script = withContext(dispatchers.io()) where you get the script on IO, and then withContext(dispatchers.main() for calling addDocumentStartJavaScript. i.e., not nesting the withContext calls.
No strong feelings though.
CDRussell approved these changes Nov 4, 2025
Merge activity
|
CrisBarreiro changed the base branch from feature/cris/derisk-webviewcompat/create-js-script to graphite-base/7036 
CrisBarreiro force-pushed the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch from 5264c26 to 759a26d Compare CrisBarreiro deleted the feature/cris/derisk-webviewcompat/configure-js-script-based-on-rc-config branch Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1211755269770777?focus=true
Description
Configure script based on remote config
Steps to test this PR
Feature 1
UI changes
n/a