fix snapshots for wkwebview to let data load

[PriyankaAngotra]

In this PR

Since ver1.15.2, snapshots have been broken for WKWebView since it no more lets the data load before taking snapshot. This has broken our(private company) tests since we want to verify that the data we pass wkwebview loads before taking a snapshot. As a result, we've been forced to stick to 1.15.0 and miss out on latest updates.
This PR reverts that change from 1.15.2. Happy to have a conversation regarding this particular fix which was done for a different issue.

[carlos-santos-anchor]

I think it was introduced here: #692

[kahseng-seek]

Our organization is also experiencing the same issue with the web view snapshot since version 1.15.2. Is there any plan to merge this? Are there any recommended workarounds we could use at the moment?

[mbrandonw]

Hi @PriyankaAngotra and @kahseng-seek, according to #692 this was needed to fix to prevent a crash:

Without this patch snapshots crash, with this patch, only the background color of the webview is snapped.

And multiple people seemed to have corroborated that. Are you saying that is not true?

[PriyankaAngotra]

Hi @mbrandonw, that's correct. From the description it seems like the patch was more of a workaround and will probably need a different solution with some debugging. With this PR, a snapshot is generated for wkwebview with the html data loaded as was my experience. The use case is very simple with terms and conditions being loaded in a wkwebview through a viewModel after which a screenshot is generated. With the previous patch fix (1.15.2), the screenshot generated is blank. The above mentioned version is what broke it as it displayed fine before.

[mbrandonw]

Hi @priyankaA87, honestly it's tough to make a call on this. We have one PR saying this changed prevents crashes and worked for them, and now we have another saying it causes problems.

Is there a reason you can't vend your own custom strategy for snapshotting web views that behaves how you want? While the library does ship a few basic snapshot strategies, in practice this has been very difficult to maintain. Multiple people can want a single strategy to behave in completely different ways, making it basically impossible to appease everyone.

[PriyankaAngotra]

I understand your position isn't easy here. I am not entirely clear on the original issue but if @teameh can provide a simple example of the original issue, that might be useful to make a judgement. As I understand, a WKWebView should be able to load a basic HTML at the very least for an acceptable screenshot to be taken which is what I tried to test.

[teameh]

Hi all and thanks for tagging me! Well the patch I added was indeed a workaround for a crashing WebView with XCode 14 and iOS 16. If this is not crashing any more without the patch, then my patch could be reverted or this PR could be applied.

However, I think it would also be good to take a look at the test case for snapping WebViews. https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Tests/SnapshotTestingTests/SnapshotTestingTests.swift#L1130-L1146 This is currently not executed on the CI so nobody is seeing that is would be failing (same for me when submitting the patch).

If I now run the test locally with or without the patch, I get a transparant image as the result instead of the expected webpage:

[mbrandonw]

Hi @teameh, FWIW we do all of our web view snapshotting on Mac when testing for the Point-Free website, and that works just fine. I'm not sure when this strategy stopped work for iOS, but this does bring me back to my previous point:

Is there a reason you can't vend your own custom strategy for snapshotting web views that behaves how you want? While the library does ship a few basic snapshot strategies, in practice this has been very difficult to maintain. Multiple people can want a single strategy to behave in completely different ways, making it basically impossible to appease everyone.

As we are seeing here, maintaining these strategies is a significant amount of work, and ultimately we want to get away from the library being the de facto source of strategies, and instead be the infrastructure that glues everything together.

And so I do want to ask again: for those having problems with a strategy provided by the library, can you try defining your own strategy that works for your use case?

[teameh]

Well there's is no specific strategy for WKWebView right? This is just using your default UI/NSView(Controller) strategy. Can we define our own strategies for webviews inside of view(controllers)?

As we are seeing here, maintaining these strategies is a significant amount of work

Yeah I completely understand. But I guess you probably included in the lib because it's quite common for apps to use some web content in their app and they want a snapshot to verify it.

I'm not sure when this strategy stopped work for iOS

Xcode 14 RC, and I haven't seen a successful iOS webview snapshot since any more, and the test in this repo is also failing. I was surprised by @PriyankaAngotra stating that it did for them.

So I think somebody should indeed dive into this and try to find out why things aren't working any more on iOS and at least proof by updating this 5y old screenshot that some webview strategy can still work on iOS.

But regardless of the strategy not working any more. I think you can apply this patch, because the crash in the test is not occurring any more as far as I can tell.

[PriyankaAngotra]

@mbrandonw Happy to merge based on above comments?

[PriyankaAngotra]

@mbrandonw Not sure you've seen my nudge above, but this will unblock us from updating to newer versions since 1.15. 🙏