accompanist icon indicating copy to clipboard operation
accompanist copied to clipboard

[WebView] Suggestion for persistent WebView upon configuration change or navigating to/from WebView composable

Open jacobmichaelis opened this issue 3 years ago • 17 comments

Describe the bug

Android Bug: When rotating the screen, or navigating to another composable and back, the WebView recomposes with no way to show an existing stored WebView.

To Reproduce

  • Implement Accompanist WebView
  • Show a web page on the mobile device
  • Rotate the screen
  • See the web page reload

Expected behavior

The website shown in the WebView doesn't reload upon configuration change/navigating back to WebView. Suggestion: I am storing my WebView on the ViewModel, but there is no way to just show the existing cached WebView I have when recomposing the WebView by Accompanist. My suggestion is that you add an existingWebView: WebView? = null to the WebView composable and then change var webView by remember { mutableStateOf<WebView?>(null) } to be mutableStateOf(existingWebView). Then in the AndroidView do webView ?: WebView(context).apply { .... I have tested this in my project by copying your code and altering it and it worked as I had hoped. This is just a suggestion, I'm sure you will have a much more brilliant way of doing what I suggested, but I hope it will be available in one way or another in the future.

Environment:

  • Android OS version: 24+
  • Device: Pixel 4a Emulator
  • Accompanist version: 0.24.7-alpha (though I'm pretty sure this is still an issue in current versions)

jacobmichaelis avatar May 27 '22 14:05 jacobmichaelis

Thanks for the report, this is definitely something we need to improve.

Just as a side note, I wouldn't store a view in the viewmodel, so be careful. That sounds like an almost certain memory leak on rotation as that view will be holding on to a context reference. The proper way to handle this is by saving and restoring state, there isn't a great way to do that right now unfortunately.

One thing that might help though, if you are in a fully Compose app you can pretty safely opt out of rotation by adding

android:configChanges="orientation|screenSize|screenLayout"

to your activity in your AndroidManifest. That should help this issue.

bentrengrove avatar Jun 08 '22 07:06 bentrengrove

@bentrengrove thanks for the advice. If I'm going to be honest, I am actually going to force portrait on the page, screen rotation was just the easiest way to illustrate the issue. The real issue I wanted to overcome was navigating to another page in the nav graph and then navigating back would cause a reload that I don't want. If you have any other suggestions on where to store that let me know, and I'm assuming you are someone who could make changes to the Accompanist library, I'd love to hear your thought process on approaching the solution to this.

jacobmichaelis avatar Jun 08 '22 15:06 jacobmichaelis

My workaround for that issue now is using a fullscreen dialog(or bottom sheet also will work I guess) nav destination instead of "composable". In that case your screen won't be disposed and webview isn't needed to be reloaded.

agent10 avatar Jun 08 '22 15:06 agent10

Hey guys, I wanted to avoid the reload of webview after navigation, I don't think storing it in ViewModel would be a proper solution too, but this is kind of a serious problem as we are currently developing a hybrid app similar to amazon(native and webview)completely with compose, the main content will be shown in webview, don't want to show it within bottom sheet or fullscreen dialog as it would be the main content to be shown to user after launch, can someone please recommend some other solution or a workaround for this if possible?

bharath1997 avatar Jun 09 '22 07:06 bharath1997

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Jul 10 '22 03:07 github-actions[bot]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Aug 10 '22 03:08 github-actions[bot]

I tried to solve this issue with caching WebView instance for each route in navigation. It worked well as I wanted, however, I'm not sure it is the best solution.

workspace avatar Aug 10 '22 04:08 workspace

@workspace can you provide the solution for this in simple code or can you elaborate on what you did?

bharath1997 avatar Aug 10 '22 17:08 bharath1997

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Sep 10 '22 04:09 github-actions[bot]

@bentrengrove (Sorry for the direct mention) The bot closed this unsolved issue, please reopen it.

nevenisnoob avatar Sep 20 '22 08:09 nevenisnoob

Hello. Any solution to this issue? This is very important for apps who handle hybrid features.

darienalvarez avatar Oct 20 '22 17:10 darienalvarez

RescueTime is also struggling with this problem, in a hybrid view. An example of smuggling a handle to webview by "caching WebView instance for each route in navigation" would be very help to see @workspace

wogg avatar Oct 27 '22 17:10 wogg

@wogg I ended creating an Activity with a WebView

<activity
            android:name=".ui.WebActivity"
            android:launchMode="singleInstance"
            android:configChanges="orientation|screenSize"
            android:theme="@style/Theme.WebActivity"
            android:exported="false" />

in the oncreate

 if (savedInstanceState != null) {
            webView.restoreState(savedInstanceState.getBundle(WEB_VIEW_KEY) ?: bundleOf())

            webView.reload()
        } else {
            webView.load(url)
        }

and also

override fun onSaveInstanceState(outState: Bundle) {
        super.onSaveInstanceState(outState)

        Timber.i("onSaveInstanceState")

        val bundle = bundleOf()
        webView.saveState(bundle)
        outState.putBundle(WEB_VIEW_KEY, bundle)
    }

darienalvarez avatar Oct 28 '22 15:10 darienalvarez

We are using jetpack compose and I am unsure how you would mix that approach in with a tabbed composable view, where the webview is on one of the tabs. Thank you for the example though.

wogg avatar Oct 28 '22 18:10 wogg

So there is a workaround now available at least for things like Pagers. You can now hoist the WebView instance above your Pager.

Very rough pseudocode, you'll probably have to improve it for your situation for instance making the WebView allocation lazy.

@Composable
fun Screen() {
   val pageCount = 5
   val context = LocalContext.current
   val webViews = remember { List(pageCount) { WebView(context) } }
   val webViewState = List(pageCount) { rememberWebViewState("YOUR_URL") }
   
   Pager(totalPages = pageCount) { page ->
      WebView(
         state = webViewState[page]
         factory = { webViews[page] }
      )
   }
}

bentrengrove avatar Oct 28 '22 20:10 bentrengrove

I used rememberSaveable to store/restore WebView state.

@Composable
fun AccompanistWebSample() {
    val initialUrl = "https://google.github.io/accompanist/web/"

    var lastUrl by rememberSaveable { mutableStateOf<String?>(null) }
    var bundle by rememberSaveable { mutableStateOf<Bundle?>(null) }

    val url = lastUrl ?: initialUrl

    val state = rememberWebViewState(url = url)

    WebView(
        state = state,
        modifier = Modifier.fillMaxSize(),
        onCreated = { webView ->
            bundle?.let {
                webView.restoreState(it)
                bundle = null
            }
        },
        onDispose = { webView ->
            bundle = Bundle().apply {
                webView.saveState(this)
            }
            lastUrl = webView.url
        }
    )
}

yanzm avatar Nov 21 '22 06:11 yanzm

Yeah, I also think it's important to use a webview in a hybird app. But now, the webview cannot save the state. Is there another way to keep webview's state?

YugeCse avatar Nov 23 '22 01:11 YugeCse

@bentrengrove Do you have any progress on this issue??

darienalvarez avatar Dec 21 '22 15:12 darienalvarez

No progress, there isn't much we can do about this from the WebView wrapper. This fix needs to come from Compose itself adding a method to do this which I am pushing for. I am keeping this issue open as I know it's a common problem.

bentrengrove avatar Jan 10 '23 02:01 bentrengrove

@bentrengrove currently I regret to choose compose for my app. There are too many scenarios where compose is very complicated to use:

  • Webviews.
  • Permission handling.
  • Fingerprint dialogs.

Those are common features in a lot of apps, it is unfortunate that google expends a lot of time in advertise a product that is far away to be complete or good enough.

darienalvarez avatar Feb 01 '23 17:02 darienalvarez

Thank you for your honest feedback @darienalvarez . I agree we definitely have room for improvement in those areas. I have passed this on to the wider team for discussion.

We also have an upstream issue for tracking the mechanism needed to fix this WebView issue. https://issuetracker.google.com/252891031

bentrengrove avatar Feb 01 '23 21:02 bentrengrove

I had this problem also. I ended up with solution, which Uses standart WebView inside composable function.

var webView: WebView? by remember {
        mutableStateOf(null)
    }
    AndroidView(
        factory = { context ->
            webView = WebView(context).apply {
                val bundle: Bundle? = savedBundle
                if (bundle != null) {
                    restoreState(bundle)
                } else {
                    loadUrl(model.webViewUrl)
                }
            }
            webView!!
        }
    )

Then I am using Disposable effect for saving state.

 var savedBundle: Bundle? by rememberSaveable {
        mutableStateOf(null)
    }
    DisposableEffect(lifecycleOwner.lifecycle) {
        val observer = LifecycleEventObserver { _, event ->
            if (event == Lifecycle.Event.ON_STOP
                || event == Lifecycle.Event.ON_PAUSE
            ) {
                val bundle = Bundle()
                webView?.saveState(bundle)
                savedBundle = bundle
            }
        }

        lifecycleOwner.lifecycle.addObserver(observer)

        onDispose {
            lifecycleOwner.lifecycle.removeObserver(observer)
        }
    }
```

musilto7 avatar Feb 08 '23 13:02 musilto7

As some mentioned, could this be a option until the real fix comes? If so I can send you a PR. https://github.com/google/accompanist/commit/98aadd9dd5385ca47aaf064fa53e9cc61745b215

alirahimpour89 avatar Feb 17 '23 14:02 alirahimpour89

@bentrengrove How long to wait for a fix? Is there a temporary solution to the problem?

anilateapp avatar Mar 26 '23 21:03 anilateapp