Fix Naver/Daum login not syncing back to the in-app WebView - #94
Fix Naver/Daum login not syncing back to the in-app WebView#94DusanBaek wants to merge 1 commit into
Conversation
Login popups were opened in the system default browser via NSWorkspace, which stores cookies in a completely separate data store from the app's WKWebView. Logging in there never reflected in the app. Popups are now presented in an in-app window using the same WKWebViewConfiguration WebKit hands to createWebViewWith(_:for:), so the login session lands in the same cookie store as the main dictionary view. The main view reloads once the popup closes.
|
I’m checking the WebKit lifecycle and session-sharing details now; I’ll post the formal review once that’s complete. |
![typeey[bot]](https://avatars.githubusercontent.com/in/3879190?s=60&v=4){kind=small}
| NSWorkspace.shared.open(url) | ||
| } | ||
| return nil | ||
| self.popupWindow?.close() |
There was a problem hiding this comment.
Popup navigation is being handled as main-WebView navigation.
popupWebView.navigationDelegate = self sends the popup's callbacks through logic that mutates the shared isLoading, injects dictionary CSS into the callback's WebView, and calls focusInput() on the main WebView. During login this can make the main loading indicator inaccurate and run main-page presentation behavior for the popup.
The popup only needs uiDelegate = self for window.close(). Either omit its navigation delegate, or guard every main-specific callback with webView === self.webView and give the popup separate navigation handling.
| createWebViewWith configuration: WKWebViewConfiguration, | ||
| for _: WKNavigationAction, | ||
| windowFeatures _: WKWindowFeatures, | ||
| ) -> WKWebView? { |
There was a problem hiding this comment.
The two user-facing login flows are still unverified.
The PR test plan leaves both Naver and Daum login checks unchecked, and compilation/launching does not establish shared cookies, JavaScript-driven closing, manual closing, or the reload that should pick up the new session.
Please complete and record both flows before merging, including successful window.close() and manual-close behavior. A local HTML-fixture test for window.open → popup closure → main-view reload would also be valuable if practical.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Allkdic/Views/DictionaryWebView.swift">
<violation number="1" location="Allkdic/Views/DictionaryWebView.swift:147">
P2: The popup WebView reuses the main dictionary Coordinator as both its navigation and UI delegate, but that coordinator is built to drive the single main WebView and owns shared state (`isLoading` binding, `lastLoadedURL`, `weak webView`). During a Naver/Daum login the popup performs multiple navigations (form loads, OAuth redirects) before it closes, and each one will now run through the same delegate callbacks that control the main view — so popup activity can clobber the main view's loading flag / last loaded URL, and the main view's navigation-policy handling will be applied to the popup's redirects. I'd suggest giving the popup its own lightweight delegate object (one that only tracks its window and closes on `window.close()` / user close) instead of pointing `navigationDelegate`/`uiDelegate` at the shared coordinator, so popup navigation events never interfere with the dictionary WebView's state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| self.popupWindow?.close() | ||
|
|
||
| let popupWebView = WKWebView(frame: NSRect(x: 0, y: 0, width: 480, height: 640), configuration: configuration) | ||
| popupWebView.navigationDelegate = self |
There was a problem hiding this comment.
P2: The popup WebView reuses the main dictionary Coordinator as both its navigation and UI delegate, but that coordinator is built to drive the single main WebView and owns shared state (isLoading binding, lastLoadedURL, weak webView). During a Naver/Daum login the popup performs multiple navigations (form loads, OAuth redirects) before it closes, and each one will now run through the same delegate callbacks that control the main view — so popup activity can clobber the main view's loading flag / last loaded URL, and the main view's navigation-policy handling will be applied to the popup's redirects. I'd suggest giving the popup its own lightweight delegate object (one that only tracks its window and closes on window.close() / user close) instead of pointing navigationDelegate/uiDelegate at the shared coordinator, so popup navigation events never interfere with the dictionary WebView's state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Allkdic/Views/DictionaryWebView.swift, line 147:
<comment>The popup WebView reuses the main dictionary Coordinator as both its navigation and UI delegate, but that coordinator is built to drive the single main WebView and owns shared state (`isLoading` binding, `lastLoadedURL`, `weak webView`). During a Naver/Daum login the popup performs multiple navigations (form loads, OAuth redirects) before it closes, and each one will now run through the same delegate callbacks that control the main view — so popup activity can clobber the main view's loading flag / last loaded URL, and the main view's navigation-policy handling will be applied to the popup's redirects. I'd suggest giving the popup its own lightweight delegate object (one that only tracks its window and closes on `window.close()` / user close) instead of pointing `navigationDelegate`/`uiDelegate` at the shared coordinator, so popup navigation events never interfere with the dictionary WebView's state.</comment>
<file context>
@@ -131,16 +132,50 @@ private struct WebView: NSViewRepresentable {
+ self.popupWindow?.close()
+
+ let popupWebView = WKWebView(frame: NSRect(x: 0, y: 0, width: 480, height: 640), configuration: configuration)
+ popupWebView.navigationDelegate = self
+ popupWebView.uiDelegate = self
+
</file context>
|
@DusanBaek, thanks for the PR. could you please address reviews and fix the CI failure? |
|
Thanks for tracking this down, @DusanBaek — the diagnosis was right, and reusing the Continuing this in #95. Your commit is cherry-picked there with authorship preserved (
Closing in favor of #95. Credit for the fix is yours. |
2 participants
Summary
NSWorkspace.shared.open, which uses a completely separate cookie store from the app's WKWebView. Logging in there never propagated back to the app, so the dictionary view always showed a logged-out state.NSWindowbacked by aWKWebViewbuilt from the exactWKWebViewConfigurationWebKit passes tocreateWebViewWith(_:for:windowFeatures:), so the login session is written to the same data store the main dictionary view uses.window.close()after a successful login, or the user closing it manually), the main dictionary WebView reloads to pick up the new session.Test plan
tuist build/ running the Debug appSummary by cubic
Fixes Naver/Daum login popups so sessions sync with the in-app
WKWebView. Popups now open in an in-app window using the sameWKWebViewConfiguration, and the main view reloads when the popup closes.webView(_:createWebViewWith:for:windowFeatures:)to show popups in anNSWindowwith the providedWKWebViewConfiguration.NSWindowDelegatehandling to close the popup and reload the main web view (windowWillClose,webViewDidClose).NSWorkspace.shared.open.Written for commit 09728e0. Summary will update on new commits.