Bug 215388

Created attachment 406407 [details]
Patch

Created attachment 406408 [details]
Patch

Comment on attachment 406408 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406408&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:355
> +@property (nonatomic, setter=_setCanUseCredentialStorage:) BOOL _canUseCredentialStorage WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

If this is intended to be constant, could we put it on WKWebViewConfiguration?  Otherwise, could we put it on WKPreferences?

Comment on attachment 406408 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406408&action=review

r=me

> Tools/TestWebKitAPI/Tests/WebKitCocoa/Preconnect.mm:102
> +    // The preconnect to the server will use the default setting of "use the credential store",
> +    // and therefore use the credential-store-blessed NSURLSession.
> +    [webView _preconnectToServer:server.request().URL];
> +    Util::run(&anyConnections);
> +    Util::spinRunLoop(10);
> +    EXPECT_FALSE(requested);
> +
> +    // Then this request will *not* use the credential store, therefore using a different NSURLSession
> +    // that doesn't know about the above preconnect, triggering a second connection to the server.
> +    webView.get()._canUseCredentialStorage = NO;
> +    [webView loadRequest:server.request()];
> +    Util::run(&requested);

This test is a little upside-down because it verifies that the *unwanted* behavior is still possible, rather than verifying that the *wanted* behavior works as expected. Can you add a test that sets _canUseCredentialStorage up front and verifies that only one connection is made if you reconnect and then connect?

> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:355
> > +@property (nonatomic, setter=_setCanUseCredentialStorage:) BOOL _canUseCredentialStorage WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> If this is intended to be constant, could we put it on
> WKWebViewConfiguration?  Otherwise, could we put it on WKPreferences?

In Safari's current behavior, this setting ends up being toggled. So, it would need to be WKPreferences. (Or maybe _WKWebsitePolicies? _WKWebsitePolicies is actually kind of a good fit -- better than the toggle because you'd get an internally consistent behavior throughout a page load automatically. But perhaps that's too much of a risk of a change in behavior.)

(In reply to Geoffrey Garen from comment #4)
> Comment on attachment 406408 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406408&action=review
> 
> r=me
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/Preconnect.mm:102
> > +    // The preconnect to the server will use the default setting of "use the credential store",
> > +    // and therefore use the credential-store-blessed NSURLSession.
> > +    [webView _preconnectToServer:server.request().URL];
> > +    Util::run(&anyConnections);
> > +    Util::spinRunLoop(10);
> > +    EXPECT_FALSE(requested);
> > +
> > +    // Then this request will *not* use the credential store, therefore using a different NSURLSession
> > +    // that doesn't know about the above preconnect, triggering a second connection to the server.
> > +    webView.get()._canUseCredentialStorage = NO;
> > +    [webView loadRequest:server.request()];
> > +    Util::run(&requested);
> 
> This test is a little upside-down because it verifies that the *unwanted*
> behavior is still possible, rather than verifying that the *wanted* behavior
> works as expected. Can you add a test that sets _canUseCredentialStorage up
> front and verifies that only one connection is made if you reconnect and
> then connect?

That's actually what the test right above it does (with my augmentation), covering the default behavior.

Testing this is a struggle because of how Safari gets into the mess.

I think verifying both the wanted *and* unwanted behavior is the best we can do from a WebKit perspective.

(In reply to Alex Christensen from comment #3)
> Comment on attachment 406408 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406408&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:355
> > +@property (nonatomic, setter=_setCanUseCredentialStorage:) BOOL _canUseCredentialStorage WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> If this is intended to be constant, could we put it on
> WKWebViewConfiguration?  Otherwise, could we put it on WKPreferences?

It's definitely not constant.

And it's definitely meant to be per-WebView, so the possibility of sharing a prefs objective makes that a no-go.

commit-queue failed to commit attachment 406408 [details] to WebKit repository.

Created attachment 406419 [details]
Patch

Committed r265525: <https://trac.webkit.org/changeset/265525>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406419 [details].

A lot of folks bump into issues with password autofill/saving (see for instance https://developer.apple.com/forums/thread/654338), especially when building an app using cordova or capacitor.

I was going to open a new ticket; however, this ticket suggests a setting exists that can make wkWebView work "as expected" in these scenarios. Reading the attached commit I haven't been able to figure out how to configure the xcodeproject; is this something capacitor (for instance) would need to do directly on the webview?