Custom delegate on WKWebView is replaced by mock when snapshotting

[teameh]

When a user sets a custom navigation delegate on a WKWebView and makes a snapshot of a view containing this web view, the navigation delegate may be replaced by a stub that makes sure the snapshotting lib knows when to snapshot the web view. This works great when users don't implement a custom WKNavigationDelegate themselves. But when a user implements a custom delegate themselves this fails.

Consider for example a custom delegate that limits the web view to github.com and opens the browser as soon as the user taps on a link outside of github.com

class GithubNavigationDelegate: NSObject, WKNavigationDelegate {
  override init() { }

  public func webView(
    _ webView: WKWebView,
    decidePolicyFor navigationAction: WKNavigationAction,
    decisionHandler: @escaping (WKNavigationActionPolicy) -> Void
  ) {
    // limit to webpages on to github.com
    guard navigationAction.request.url?.host == "github.com" else {
      decisionHandler(.cancel)
      // Warn user and open url in browser instead
      return
    }

    decisionHandler(.allow)
  }
}


webview.navigationDelegate = GithubNavigationDelegate()

This can't be snapshot tested at the moment because the delegate is removed as soon as a snapshot is made.

I think this could be fixed by retaining the original delegate, implementing all the possible delegate methods and forwarding all the calls to the original delegate. I'll see if I can draft up a PR. Please let me know if you think of another way of dealing with this issue.

Finally, many thanks for all the work on this great lib 👏🏼 so far, hopefully I can contribute back a bit.

[stephencelis]

That solution sounds great to us! Thanks for raising the issue and we'd love a PR 😄

[teameh]

Thanks! That's quick. I'm running the tests on master locally but got lot of failing tests.. any clues?
I'm already running on iPhone 11 Pro Max (13.7)

[teameh]

Also, do you know why this is necessary?

https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Sources/SnapshotTesting/Common/View.swift#L659-L661

    webView.evaluateJavaScript("document.readyState") { _, _ in
      self.didFinish()
    }

Why execute that javascript, but then no do something with the result. Do you execute it because webkit wait with executing it until the page is loaded? What about a page that never loads, for example a 404?

[teameh]

This is harder than I thought because of all the optional methods. You'll need a couple of extra tests as well. PR might take some time.

[Sherlouk]

@teameh Could you do something with forwardingTarget to do without overriding every single method?

This is a partial example from one of my projects 😄 Obviously change the delegate/selector for the one needed by this project. It'll forward every method over automatically.

public final class NetworkProxyDelegate: NSObject, URLSessionTaskDelegate, URLSessionDataDelegate {

    private weak var actualDelegate: URLSessionTaskDelegate?
    private let interceptedSelectors: Set<Selector>

    public init(delegate: URLSessionTaskDelegate?) {
        actualDelegate = delegate
        interceptedSelectors = [
            #selector(URLSessionTaskDelegate.urlSession(_:task:didCompleteWithError:)),
        ]
    }

    // MARK: - URLSessionTaskDelegate

    public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
        // custom stuffs

        actualDelegate?.urlSession?(session, task: task, didCompleteWithError: error)
    }

    // MARK: - Proxy
    
    override public func responds(to aSelector: Selector!) -> Bool {
        if interceptedSelectors.contains(aSelector) {
            return true
        }

        return (actualDelegate?.responds(to: aSelector) ?? false) || super.responds(to: aSelector)
    }

    override public func forwardingTarget(for aSelector: Selector!) -> Any? {
        if interceptedSelectors.contains(aSelector) {
            return nil
        }

        return actualDelegate
    }
}

[teameh]

@Sherlouk that's a great idea! I've been fighting with this code for a while now and I think I got something that works.

Regarding the line:

return (actualDelegate?.responds(to: aSelector) ?? false) || super.responds(to: aSelector)

The docs say:

You cannot test whether an object inherits a method from its superclass by sending responds(to:) to the object using the super keyword. This method will still be testing the object as a whole, not just the superclass’s implementation. Therefore, sending responds(to:) to super is equivalent to sending it to self. Instead, you must invoke the NSObject class method instancesRespond(to:) directly on the object’s superclass, as illustrated in the following code fragment.

So I think you should use NSObject here:

return (actualDelegate?.responds(to: aSelector) ?? false) || NSObject.responds(to: aSelector)

[teameh]

See PR. It might needs to be tweaked a bit more, but it's a start. Thanks for pointing me to responds(to:) @Sherlouk. This is exactly why I love to spend some time on open source projects. I learned a lot today 🙌 .

[Sherlouk]

The docs say

Have you got a link to that one please? We've used this code for a while and it's been working fine for us (though admittedly we're not heavy users of the delegate!) - would love to patch this upstream in our project too.

Glad it was helpful and you learned something new 😄 Everyday is a learning opportunity

[teameh]

😄

Sure. https://developer.apple.com/documentation/objectivec/nsobjectprotocol/1418583-responds#discussion

These are the docs you get if you option-click the responds(to:) method

[teameh]

@stephencelis anything I can do to help move this forward?

[EricDeRegter]

I ran into the same issue.
@teameh any update on this?

[teameh]

@stephencelis anything I can do to help get this merged?

[teameh]

For future ref: fixed and merged in #443