================================================================================================ PROPOSAL - [MINOR] functional - p2j.sso_reauth.js handleSsoReauth The expiry reload raises the leave-confirmation, and "Stay" freezes the overlay ================================================================================================ REVIEW FINDING (abridged) "The expiry path calls window.location.reload() while p2j.socket.js's window.onbeforeunload is still armed [...] returns the 'ANY PENDING CHANGES WILL BE LOST!' confirmation whenever p2j.isGui && !exitTheApplication [...] If the user picks 'Stay', the reload is aborted after clearInterval(countdownInterval); countdownInterval = null; has already run, leaving the overlay frozen at 'Confirming sign-in...' with no further ticks." FILES src/com/goldencode/p2j/ui/client/driver/web/res/p2j.sso_reauth.js src/com/goldencode/p2j/ui/client/driver/web/res/p2j.socket.js ------------------------------------------------------------------------------------------------ 1. ROOT CAUSE ------------------------------------------------------------------------------------------------ Two independent mistakes on the same three lines. (a) The prompt. me.init() installs window.onbeforeunload (p2j.socket.js:6896) unconditionally at page load, and it returns the confirmation string whenever p2j.isGui && !exitTheApplication. At re-auth expiry no logout has run, so exitTheApplication is still false - the only assignments setting it true are doRedirectToLogoutPage() and onpagehide, and onpagehide fires *after* beforeunload. Nothing ever clears the handler. Sticky user activation is satisfied (a session that reached SSO token expiry has had canvas input; clicking Re-login alone sets it), so the browser prompts. This prompt is newly introduced by this diff: the deleted showSessionExpiredPage() rewrote document.body and never unloaded, so the old code could not hit it. Note the codebase's own convention for a deliberate scripted navigation is to set exitTheApplication = true first, which is exactly what the overlay's own Logout button does via doRedirectToLogoutPage. This reload is the one navigation that skips it. (b) The teardown ordering. The branch runs clearInterval(countdownInterval); countdownInterval = null; countdownDiv.textContent = 'Confirming sign-in...'; window.location.reload(); reload() only *requests* a navigation; it returns, and the navigation can be cancelled. By then the interval is already gone, so on "Stay" nothing ticks again: the overlay is stuck on "Confirming sign-in...", cleanupReauthUi never runs, the modal keydown trap stays armed and the socket keeps echoing the heartbeat. The Logout button still works as a manual escape, so the user is not trapped, but the automatic recovery is silently defeated. ------------------------------------------------------------------------------------------------ 2. PROPOSED FIX ------------------------------------------------------------------------------------------------ 2.1 Suppress the prompt for this reload ----------------------------------------- Reuse the internalReload flag introduced in proposal-issue-minor-functional-2 section 2.1 - the same signal that stops onpagehide arming quit-on-reload should stop beforeunload prompting, since both ask the same question ("is the user leaving?") and the answer is the same "no": window.onbeforeunload = function(evt) { - if (p2j.isGui && !exitTheApplication) + if (p2j.isGui && !exitTheApplication && !internalReload) { var confirmationMessage = "ANY PENDING CHANGES WILL BE LOST!\n" + ... If minor-functional-2 is not taken, this proposal must introduce the flag itself; the two are better landed together. 2.2 Keep the countdown alive across a cancelled navigation ------------------------------------------------------------ Do not tear down before requesting the navigation. Let the interval keep running and make the branch idempotent instead: + if (reloadRequested) + { + // the navigation was requested and is either in flight or was cancelled by the user; + // do not request it again on every tick + return; + } + + reloadRequested = true; - clearInterval(countdownInterval); - countdownInterval = null; ... countdownDiv.textContent = 'Confirming sign-in...'; + p2j.socket.beginInternalReload(); window.location.reload(); return; with a module variable beside the others: + /** True once the expiry reload has been requested; keeps the tick from re-requesting it. */ + var reloadRequested = false; The interval is left running deliberately. On a successful navigation it dies with the page; on "Stay" it keeps ticking, which is what lets 2.3 offer the user a way forward. 2.3 Give the user something to do if they decline --------------------------------------------------- With the interval still alive, a declined navigation should say so rather than sit on "Confirming sign-in...": + countdownDiv.textContent = 'Sign-in could not be confirmed. Use Logout to continue.'; set on the tick *after* reloadRequested is already true (i.e. in the early-return branch above), so it only appears once the navigation has demonstrably not happened. The Logout button is already enabled and already works. An alternative worth considering is to leave the overlay alone and simply not offer a reload at all on this path - see proposal-issue-minor-functional-2 section 2.2, where the server reports the outcome over the existing socket and no navigation is needed. ------------------------------------------------------------------------------------------------ 3. RELATIONSHIP TO OTHER PROPOSALS ------------------------------------------------------------------------------------------------ proposal-issue-minor-functional-2 introduces internalReload and owns the sessionStorage half of the same reload. Prerequisite for 2.1. proposal-issue-minor-functional-4 closes the orphaned popup on the same branch. All three edit the same expiry branch; apply as one change. ------------------------------------------------------------------------------------------------ 4. RISK AND VERIFICATION ------------------------------------------------------------------------------------------------ Risk: low. 2.1 narrows a prompt that should never have fired on this path. 2.2 removes a teardown that was premature; the new reloadRequested guard is what prevents the reload being re-requested once per second, so do not take 2.2 without it. Verification 1. Trigger SSO re-auth in the GUI client, interact with the canvas first so sticky activation is set, and let the countdown expire. Expected: no leave-confirmation dialog, page reloads straight away. Pre-fix the dialog appears. 2. Pre-fix build, same test, choose "Stay": confirm the frozen "Confirming sign-in..." overlay. Post-fix, force the same state (e.g. temporarily re-enable the prompt) and confirm the tick still runs and the message changes to the actionable text from 2.3. 3. Confirm the Logout button still tears down cleanly from the post-reload-request state. 4. Confirm the prompt still appears for an ordinary user-initiated close or F5 with unsaved work, i.e. internalReload has not disabled it globally.