webxdc: "onunload" does not work

[r10s]

maybe it is just a bug in the testing tool (https://github.com/webxdc/webxdc-test/pull/1), maybe a switch is missing - however, "onunload" is an important thing needed for sendUpdate()-optimizations, we should make that work somehow.

[r10s]

seems to be missing by definition.

https://stackoverflow.com/questions/51940005/wkwebview-localstorage-becomes-null-on-window-close shows a way to trigger onunload:

func webViewDidClose(_ webView: WKWebView) {
    // Trigger the unload event before closing the web view
    webView.evaluateJavaScript("dispatchEvent(new Event('unload'));")

    // Give the content some time to respond to the event, and then destroy the web view
    DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(500)) {
        self.webView?.removeFromSuperview();
        self.webView = nil
    }
}

but that seems to be tricky, timeouts, the author also says localStorage is already null. that seems to need some tests and investigations; at https://github.com/webxdc/webxdc-test/pull/1 we have a test for onunload meanwhile.

EDIT: reading a bit more, webViewDidClose seems to be just wrong here, it is triggered when the webview is closed, this is too late.

• we should try to trigger onunload somewhere in WebxdcViewController.viewWillDisappear and detach the wkWebView so that it survives the closing of its parent and survives another second until we can assume onload has finished.

• moreover, onunload should also be triggered when another page is loaded internally. for testing, in the webxdc-test-tool, "Link to internal HTML page" and then "« Go Back" should increase the unload counter:
  

[cyBerta]

according to https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event#usage_notes developers should not rely on unload events, since they are not well supported, especially in the mobile context.

Alternertively https://developer.mozilla.org/en-US/docs/Web/API/Document/visibilitychange_event is proposed in the same documentation.

If we can use an approach on all platforms that doesn't require fighting against the intended platform specific behavior (in case of iOS, but maybe also Android), I would suggest to try that route.

[r10s]

if visibilityChanged works on all platforms, we can go for that, however, i have not tried it on ios nor it is part of the webxdc-test yet.

wrt onunload: the advantage is, that it is very well known since 20+ years, esp. by ppl not that deep into "modern" js. probably tons of js code just uses it and could be used without adaption in webxdc then - also in mozilla browsers, these events work ;)

so, theory aside: we should test in practise how well visibilityChanged is supported in the webviews we're using before we can decide on that.

[cyBerta]

PR to test visibilityChange on all platforms: https://github.com/webxdc/webxdc-test/pull/2

[cyBerta]

@r10s shall we close this issue?

[r10s]

+1, we have meanwhile documented at https://github.com/deltachat/deltachat-core-rust/blob/master/draft/webxdc-dev-reference.md#other-apis-and-tags-usage-hints that visibilitychange should be used and unload is discouraged.