Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save all retrieved cookies and get cookies using javascript #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kheldiente
Copy link

@kheldiente kheldiente commented May 21, 2020

@kheldiente kheldiente changed the title Save all retrieved cookies and evaluate cookies using javascript Save all retrieved cookies and get cookies using javascript May 21, 2020
@kheldiente kheldiente force-pushed the 17-Missing-cookie-values-in-iOS-10-or-lower branch from 5addbc6 to cba1960 Compare May 21, 2020 15:20
Copy link
Owner

@Kofktu Kofktu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a separate value called recordCookies,
store the cookie values in HTTPCookieStorage
In the service, it would be better to know through callback(onUpdateCookieStroage).

@@ -33,6 +33,8 @@ open class WKCookieWebView: WKWebView {
// The closure where cookie information is called at update time
@objc public var onUpdateCookieStorage: ((WKCookieWebView) -> Void)?

@objc public var recordedCookies: [HTTPCookie] = [];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables managed only internally
I think private would be better than public.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove ;

@objc public func deleteRecordedCookie(_ cookie: HTTPCookie) {
recordedCookies.removeAll(where: { $0.name == cookie.name })
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also public -> private and remove @objc

@kheldiente
Copy link
Author

kheldiente commented May 26, 2020

Instead of creating a separate value called recordCookies,
store the cookie values in HTTPCookieStorage
In the service, it would be better to know through callback(onUpdateCookieStroage).

Hi @Kofktu , as per testing on my end, setting cookies to HTTPCookieStorage does not work in iOS 10 or lower. I believe this has something to do with evaluateDocumentCookies(webView: WKWebView) having dummy properties in cookies. Also, this validates the intention for storing it in a dedicated array and enabling clients to browse all cookies retrieved by WKCookieWebView, hence it is public.

For deleteRecordedCookie(_ cookie: HTTPCookie), I'll remove that one. Commit for improvement is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing cookie values in iOS 10 or lower
2 participants