Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebView] WebViewState not updated within AccompanistWebViewClient #1164

Closed
jbh6gc opened this issue May 12, 2022 · 2 comments · Fixed by #1166
Closed

[WebView] WebViewState not updated within AccompanistWebViewClient #1164

jbh6gc opened this issue May 12, 2022 · 2 comments · Fixed by #1166
Assignees

Comments

@jbh6gc
Copy link

jbh6gc commented May 12, 2022

Describe the bug

If a WebView composable is passed a new instance of WebViewState after the initial composition of the WebView, the AccompanistWebViewClient and AccompanistWebChromeClient instances passed in to the WebView composable do not get updated to use the new WebViewState instance. This results in the code calling the WebViewComposable using a different instance of WebViewState than what the clients are using. In my case, the LoadingState on the new instance of WebViewState is stuck as Loading and is never changed to Finished. From debugging I can see that this is because the AccompanistWebViewClient is updating the initial instance of the WebViewState instead of the new one.

To Reproduce

Steps to reproduce the behavior:

  1. Instantiate a WebView with a WebViewState (created with the provided remember functions) and implementation of AccompanistWebViewClient passed in as parameters.
  2. After initial composition of the WebView, change the data for the WebViewState so that a new instance is created and passed to the WebView composable.
  3. Check the LoadingState of your new WebState and observe that it is out of sync with the one that the AccompanistWebViewClient is referencing.

Here's a code snippet to illustrate my setup that lead to this as well:

val webViewState = rememberWebViewStateWithHTMLData(
   data = internalHtml,
   baseUrl = webViewUrl
)

 WebView(
     state = webViewState,
     captureBackPresses = false,
     onCreated = {
                webView = it
                it.clearCache(true)
                it.clearHistory()
                it.settings.javaScriptEnabled = true
                it.addJavascriptInterface(webHandler, webHandler.interfaceName)
                Timber.d("Created WebView")
     },
     client = webViewClient,
     chromeClient = remember { MyWebChromeClient() },
)

// We change internalHtml at some point which results in the new webViewState we're referencing here no longer reflecting the LoadingState of the WebView accurately.
if(!webViewState.isLoading) {
   applyJavascript(webView)
}

Expected behavior

I would expect that either the web view clients are updated with the new instance of WebViewState on each composition so the referenced states stay in sync or alternatively, the rememberWebViewState* functions could update the content field of the WebViewState instance instead of returning a new instance of WebViewState if that data changes. The latter behavior may be unexpected by consumers since remember functions typically return new instances when the passed-in data changes.

Environment:

  • Android OS version: Observed this on API 31, 30, and 29
  • Device: Observed on a Pixel 6, a Nexus 9 emulator, and a Pixel 5 emulator
  • Accompanist version: 0.24.7-alpha

Additional context

I tried to include all of the same information here, but here's my conversation on this from the Kotlinlang Slack space: https://kotlinlang.slack.com/archives/CJLTWPH7S/p1652281163585679.

@jbh6gc
Copy link
Author

jbh6gc commented May 12, 2022

This seems to be due to the WebViewState being set on the clients here. This factory lambda is only called on the initial creation of the Android view as per the AndroidView docs. So, if a different WebViewState instance is passed in after that first composition, it won't be updated on the clients. Presumably, if it is updated on the clients there'd have to be some sort of reconciliation between the state passed in and the state currently on the clients.

@jbh6gc
Copy link
Author

jbh6gc commented May 12, 2022

FYI, I worked around this by doing the following instead of using the provided rememberWebVIewState* functions:

val webViewData = remember(webViewHtml, webViewUrl) {
        WebContent.Data(internalHtml, webViewUrl)
    }
    // Only create this state instance once since the WebView Composable does not fully update for new instances
    val webViewState = remember { WebViewState(webViewData) }
    // When the webViewData changes, set it on the webViewState. Content is a mutableState field so updating it will trigger the appropriate recompositons.
    LaunchedEffect(webViewData) {
        webViewState.content = webViewData
    }

This updates the content field without changing the state instance and since content is a MutableState this triggers the needed recompositions internally within WebView.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants