b8b745a fix(ui): keep JS dialog focus, track focus via controller, handle blob origins · dumber

M internal/ui/component/jsdialog_popup.go +36 -1


@@ -4,6 +4,7 @@ import (
4	4		"net/url"
5	5		"strings"
6	6		"sync"
	7	+	"sync/atomic"
7	8
8	9		"github.com/bnema/puregotk/v4/gdk"
9	10		"github.com/bnema/puregotk/v4/gtk"

@@ -52,6 +53,12 @@ func jsDialogOriginLabel(origin string) string {
52	53		return "This page"
53	54		}
54	55		u, err := url.Parse(origin)
	56	+	// blob:/filesystem: URLs carry the real origin in the opaque part.
	57	+	if err == nil && u.Host == "" && (u.Scheme == "blob" \|\| u.Scheme == "filesystem") && u.Opaque != "" {
	58	+	if inner, innerErr := url.Parse(u.Opaque); innerErr == nil && inner.Host != "" {
	59	+	u = inner
	60	+	}
	61	+	}
55	62		if err != nil \|\| u.Host == "" {
56	63		if err == nil && u.Scheme != "" {
57	64		return truncateRunesLeft(u.Scheme+":", jsDialogMaxHostChars)

@@ -122,6 +129,8 @@ type JSDialogPopup struct {
122	129
123	130		parent layout.OverlayWidget
124	131		controller *gtk.EventControllerKey
	132	+	focusCtrl *gtk.EventControllerFocus
	133	+	hasFocus atomic.Bool // keyboard focus is inside the popup (enter/leave)
125	134		signals []popupSignal
126	135
127	136		mu sync.Mutex

@@ -214,6 +223,13 @@ func (p *JSDialogPopup) Show(req port.JSDialogRequest, respond func(ok bool, inp
214	223		}
215	224		}
216	225
	226	+	// Visible reports whether a dialog is currently shown.
	227	+	func (p *JSDialogPopup) Visible() bool {
	228	+	p.mu.Lock()
	229	+	defer p.mu.Unlock()
	230	+	return p.visible
	231	+	}
	232	+
217	233		// RequestFocus marks the dialog as the focus owner and grabs focus if shown.
218	234		// Called when the owning pane becomes active.
219	235		func (p *JSDialogPopup) RequestFocus() {

@@ -265,7 +281,8 @@ func (p JSDialogPopup) close(answer bool) {
265	281		hidden := p.onHidden
266	282		p.mu.Unlock()
267	283
268		-	hadFocus := p.outerBox.GetFocusChild() != nil
	284	+	// Read before hiding: hiding a focused widget fires "leave".
	285	+	hadFocus := p.hasFocus.Load()
269	286		text := ""
270	287		if answer != nil && *answer && p.input.GetVisible() {
271	288		text = p.input.GetText()

@@ -304,6 +321,10 @@ func (p *JSDialogPopup) Destroy() {
304	321		p.outerBox.RemoveController(&p.controller.EventController)
305	322		p.controller = nil
306	323		}
	324	+	if p.focusCtrl != nil && p.outerBox != nil {
	325	+	p.outerBox.RemoveController(&p.focusCtrl.EventController)
	326	+	p.focusCtrl = nil
	327	+	}
307	328		p.retainedCallbacks = nil
308	329		}
309	330

@@ -444,6 +465,20 @@ func (p *JSDialogPopup) attachKeyController() {
444	465		p.outerBox.AddController(&controller.EventController)
445	466		p.controller = controller
446	467
	468	+	// Track focus-within so close() knows whether the dialog owned focus
	469	+	// (GetFocusChild would leak a reference).
	470	+	if fc := gtk.NewEventControllerFocus(); fc != nil {
	471	+	enterCb := func(gtk.EventControllerFocus) { p.hasFocus.Store(true) }
	472	+	leaveCb := func(gtk.EventControllerFocus) { p.hasFocus.Store(false) }
	473	+	p.retainedCallbacks = append(p.retainedCallbacks, enterCb, leaveCb)
	474	+	p.signals = append(p.signals,
	475	+	popupSignal{fc, fc.ConnectEnter(&enterCb)},
	476	+	popupSignal{fc, fc.ConnectLeave(&leaveCb)},
	477	+	)
	478	+	p.outerBox.AddController(&fc.EventController)
	479	+	p.focusCtrl = fc
	480	+	}
	481	+
447	482		// A dialog shown while its tab was in the background gets focus once mapped.
448	483		mapCb := func(gtk.Widget) {
449	484		p.mu.Lock()

 	"net/url"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"github.com/bnema/puregotk/v4/gdk"
 	"github.com/bnema/puregotk/v4/gtk"
 		return "This page"
 	}
 	u, err := url.Parse(origin)
+	// blob:/filesystem: URLs carry the real origin in the opaque part.
+	if err == nil && u.Host == "" && (u.Scheme == "blob" || u.Scheme == "filesystem") && u.Opaque != "" {
+		if inner, innerErr := url.Parse(u.Opaque); innerErr == nil && inner.Host != "" {
+			u = inner
+		}
+	}
 	if err != nil || u.Host == "" {
 		if err == nil && u.Scheme != "" {
 			return truncateRunesLeft(u.Scheme+":", jsDialogMaxHostChars)
 	parent     layout.OverlayWidget
 	controller *gtk.EventControllerKey
+	focusCtrl  *gtk.EventControllerFocus
+	hasFocus   atomic.Bool // keyboard focus is inside the popup (enter/leave)
 	signals    []popupSignal
 	mu        sync.Mutex
 	}
 }
+// Visible reports whether a dialog is currently shown.
+func (p *JSDialogPopup) Visible() bool {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return p.visible
+}
 // RequestFocus marks the dialog as the focus owner and grabs focus if shown.
 // Called when the owning pane becomes active.
 func (p *JSDialogPopup) RequestFocus() {
 	hidden := p.onHidden
 	p.mu.Unlock()
-	hadFocus := p.outerBox.GetFocusChild() != nil
+	// Read before hiding: hiding a focused widget fires "leave".
+	hadFocus := p.hasFocus.Load()
 	text := ""
 	if answer != nil && *answer && p.input.GetVisible() {
 		text = p.input.GetText()
 		p.outerBox.RemoveController(&p.controller.EventController)
 		p.controller = nil
 	}
+	if p.focusCtrl != nil && p.outerBox != nil {
+		p.outerBox.RemoveController(&p.focusCtrl.EventController)
+		p.focusCtrl = nil
+	}
 	p.retainedCallbacks = nil
 }
 	p.outerBox.AddController(&controller.EventController)
 	p.controller = controller
+	// Track focus-within so close() knows whether the dialog owned focus
+	// (GetFocusChild would leak a reference).
+	if fc := gtk.NewEventControllerFocus(); fc != nil {
+		enterCb := func(gtk.EventControllerFocus) { p.hasFocus.Store(true) }
+		leaveCb := func(gtk.EventControllerFocus) { p.hasFocus.Store(false) }
+		p.retainedCallbacks = append(p.retainedCallbacks, enterCb, leaveCb)
+		p.signals = append(p.signals,
+			popupSignal{fc, fc.ConnectEnter(&enterCb)},
+			popupSignal{fc, fc.ConnectLeave(&leaveCb)},
+		)
+		p.outerBox.AddController(&fc.EventController)
+		p.focusCtrl = fc
+	}
 	// A dialog shown while its tab was in the background gets focus once mapped.
 	mapCb := func(gtk.Widget) {
 		p.mu.Lock()

M internal/ui/component/jsdialog_popup_test.go +15 -0


@@ -19,6 +19,21 @@ func TestJSDialogOriginLabel(t *testing.T) {
19	19		assert.LessOrEqual(t, len([]rune(long)), jsDialogMaxHostChars+1)
20	20		}
21	21
	22	+	func TestJSDialogOriginLabelOpaqueSchemes(t *testing.T) {
	23	+	cases := map[string]string{
	24	+	"blob:https://example.com/3f1c-uuid": "example.com",
	25	+	"blob:http://sub.example.com:8080/uuid": "sub.example.com:8080",
	26	+	"filesystem:https://example.com/temporary/file.txt": "example.com",
	27	+	"blob:null/uuid": "blob:",
	28	+	"blob:": "blob:",
	29	+	"data:text/html,<b>x</b>": "data:",
	30	+	"javascript:alert(1)": "javascript:",
	31	+	}
	32	+	for in, want := range cases {
	33	+	assert.Equal(t, want, jsDialogOriginLabel(in), in)
	34	+	}
	35	+	}
	36	+
22	37		func TestJSDialogOriginLabelKeepsRegistrableDomain(t *testing.T) {
23	38		// A long attacker-controlled prefix must not push the real domain out.
24	39		label := jsDialogOriginLabel("https://" + strings.Repeat("paypal.com.", 20) + "evil.test/")


@@ -19,6 +19,21 @@ func TestJSDialogOriginLabel(t *testing.T) {
19	assert.LessOrEqual(t, len([]rune(long)), jsDialogMaxHostChars+1)	19		assert.LessOrEqual(t, len([]rune(long)), jsDialogMaxHostChars+1)
20	}	20		}
21		21
		22	+	func TestJSDialogOriginLabelOpaqueSchemes(t *testing.T) {
		23	+	cases := map[string]string{
		24	+	"blob:https://example.com/3f1c-uuid": "example.com",
		25	+	"blob:http://sub.example.com:8080/uuid": "sub.example.com:8080",
		26	+	"filesystem:https://example.com/temporary/file.txt": "example.com",
		27	+	"blob:null/uuid": "blob:",
		28	+	"blob:": "blob:",
		29	+	"data:text/html,<b>x</b>": "data:",
		30	+	"javascript:alert(1)": "javascript:",
		31	+	}
		32	+	for in, want := range cases {
		33	+	assert.Equal(t, want, jsDialogOriginLabel(in), in)
		34	+	}
		35	+	}
		36	+
22	func TestJSDialogOriginLabelKeepsRegistrableDomain(t *testing.T) {	37		func TestJSDialogOriginLabelKeepsRegistrableDomain(t *testing.T) {
23	// A long attacker-controlled prefix must not push the real domain out.	38		// A long attacker-controlled prefix must not push the real domain out.
24	label := jsDialogOriginLabel("https://" + strings.Repeat("paypal.com.", 20) + "evil.test/")	39		label := jsDialogOriginLabel("https://" + strings.Repeat("paypal.com.", 20) + "evil.test/")

M internal/ui/component/pane_view.go +22 -0


@@ -31,6 +31,7 @@ type PaneView struct {
31	31		jsDialog jsDialogView // JS alert/confirm/prompt/beforeunload overlay
32	32		jsDialogWidget layout.Widget // overlay wrapper of jsDialog, for removal
33	33		newJSDialog func(layout.OverlayWidget) jsDialogView
	34	+	cleanedUp bool // Cleanup ran: no new JS dialog may be created
34	35		paneID entity.PaneID
35	36		isActive bool
36	37

@@ -219,11 +220,21 @@ func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) {
219	220
220	221		// GrabFocus attempts to focus the WebView.
221	222		// Returns true if focus was successfully grabbed.
	223	+	//
	224	+	// While a JS dialog is open it owns keyboard focus: activation paths call
	225	+	// SetActive(true) (which requests dialog focus) and then GrabFocus, which must
	226	+	// not steal focus back to the page.
222	227		func (pv *PaneView) GrabFocus() bool {
223	228		pv.mu.RLock()
224	229		wv := pv.webViewWidget
	230	+	popup := pv.jsDialog
225	231		pv.mu.RUnlock()
226	232
	233	+	if popup != nil && popup.Visible() {
	234	+	popup.RequestFocus()
	235	+	return true
	236	+	}
	237	+
227	238		if wv == nil {
228	239		return false
229	240		}

@@ -556,8 +567,14 @@ func (pv *PaneView) IsVimMode() bool {
556	567		// Cleanup removes the WebView widget from the overlay and clears references.
557	568		// This must be called before destroying the WebView to ensure proper GTK cleanup.
558	569		// After calling Cleanup, the PaneView should not be reused.
	570	+	//
	571	+	// An open JS dialog is answered with cancel ("Stay" for beforeunload). That is
	572	+	// an accepted trade-off: WorkspaceView.Rebuild (split/close/consume/reattach)
	573	+	// cleans up views whose WebView survives, so such a dialog is canceled rather
	574	+	// than carried over to the rebuilt view. Canceling is the safe default.
559	575		func (pv *PaneView) Cleanup() {
560	576		pv.mu.Lock()
	577	+	pv.cleanedUp = true
561	578		// The WebView may outlive this view (workspace rebuild), so an open JS
562	579		// dialog must be answered, not dropped. The hidden callback re-locks
563	580		// pv.mu, so detach it here and dismiss after unlocking.

@@ -625,6 +642,7 @@ type jsDialogView interface {
625	642		Hide()
626	643		Dismiss(ok bool)
627	644		RequestFocus()
	645	+	Visible() bool
628	646		SetOnHidden(fn func(hadFocus bool))
629	647		Destroy()
630	648		}

@@ -680,6 +698,10 @@ func (pv *PaneView) ensureJSDialog() jsDialogView {
680	698		// popup is unavailable.
681	699		func (pv *PaneView) ShowJSDialog(req port.JSDialogRequest, respond func(ok bool, input string), uiScale float64) bool {
682	700		pv.mu.Lock()
	701	+	if pv.cleanedUp {
	702	+	pv.mu.Unlock()
	703	+	return false
	704	+	}
683	705		popup := pv.ensureJSDialog()
684	706		focus := pv.isActive
685	707		pv.mu.Unlock()


@@ -31,6 +31,7 @@ type PaneView struct {
31	jsDialog jsDialogView // JS alert/confirm/prompt/beforeunload overlay	31		jsDialog jsDialogView // JS alert/confirm/prompt/beforeunload overlay
32	jsDialogWidget layout.Widget // overlay wrapper of jsDialog, for removal	32		jsDialogWidget layout.Widget // overlay wrapper of jsDialog, for removal
33	newJSDialog func(layout.OverlayWidget) jsDialogView	33		newJSDialog func(layout.OverlayWidget) jsDialogView
		34	+	cleanedUp bool // Cleanup ran: no new JS dialog may be created
34	paneID entity.PaneID	35		paneID entity.PaneID
35	isActive bool	36		isActive bool
36		37

@@ -219,11 +220,21 @@ func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) {
219		220
220	// GrabFocus attempts to focus the WebView.	221		// GrabFocus attempts to focus the WebView.
221	// Returns true if focus was successfully grabbed.	222		// Returns true if focus was successfully grabbed.
		223	+	//
		224	+	// While a JS dialog is open it owns keyboard focus: activation paths call
		225	+	// SetActive(true) (which requests dialog focus) and then GrabFocus, which must
		226	+	// not steal focus back to the page.
222	func (pv *PaneView) GrabFocus() bool {	227		func (pv *PaneView) GrabFocus() bool {
223	pv.mu.RLock()	228		pv.mu.RLock()
224	wv := pv.webViewWidget	229		wv := pv.webViewWidget
		230	+	popup := pv.jsDialog
225	pv.mu.RUnlock()	231		pv.mu.RUnlock()
226		232
		233	+	if popup != nil && popup.Visible() {
		234	+	popup.RequestFocus()
		235	+	return true
		236	+	}
		237	+
227	if wv == nil {	238		if wv == nil {
228	return false	239		return false
229	}	240		}

@@ -556,8 +567,14 @@ func (pv *PaneView) IsVimMode() bool {
556	// Cleanup removes the WebView widget from the overlay and clears references.	567		// Cleanup removes the WebView widget from the overlay and clears references.
557	// This must be called before destroying the WebView to ensure proper GTK cleanup.	568		// This must be called before destroying the WebView to ensure proper GTK cleanup.
558	// After calling Cleanup, the PaneView should not be reused.	569		// After calling Cleanup, the PaneView should not be reused.
		570	+	//
		571	+	// An open JS dialog is answered with cancel ("Stay" for beforeunload). That is
		572	+	// an accepted trade-off: WorkspaceView.Rebuild (split/close/consume/reattach)
		573	+	// cleans up views whose WebView survives, so such a dialog is canceled rather
		574	+	// than carried over to the rebuilt view. Canceling is the safe default.
559	func (pv *PaneView) Cleanup() {	575		func (pv *PaneView) Cleanup() {
560	pv.mu.Lock()	576		pv.mu.Lock()
		577	+	pv.cleanedUp = true
561	// The WebView may outlive this view (workspace rebuild), so an open JS	578		// The WebView may outlive this view (workspace rebuild), so an open JS
562	// dialog must be answered, not dropped. The hidden callback re-locks	579		// dialog must be answered, not dropped. The hidden callback re-locks
563	// pv.mu, so detach it here and dismiss after unlocking.	580		// pv.mu, so detach it here and dismiss after unlocking.

@@ -625,6 +642,7 @@ type jsDialogView interface {
625	Hide()	642		Hide()
626	Dismiss(ok bool)	643		Dismiss(ok bool)
627	RequestFocus()	644		RequestFocus()
		645	+	Visible() bool
628	SetOnHidden(fn func(hadFocus bool))	646		SetOnHidden(fn func(hadFocus bool))
629	Destroy()	647		Destroy()
630	}	648		}

@@ -680,6 +698,10 @@ func (pv *PaneView) ensureJSDialog() jsDialogView {
680	// popup is unavailable.	698		// popup is unavailable.
681	func (pv *PaneView) ShowJSDialog(req port.JSDialogRequest, respond func(ok bool, input string), uiScale float64) bool {	699		func (pv *PaneView) ShowJSDialog(req port.JSDialogRequest, respond func(ok bool, input string), uiScale float64) bool {
682	pv.mu.Lock()	700		pv.mu.Lock()
		701	+	if pv.cleanedUp {
		702	+	pv.mu.Unlock()
		703	+	return false
		704	+	}
683	popup := pv.ensureJSDialog()	705		popup := pv.ensureJSDialog()
684	focus := pv.isActive	706		focus := pv.isActive
685	pv.mu.Unlock()	707		pv.mu.Unlock()

M internal/ui/component/pane_view_jsdialog_test.go +37 -0


@@ -29,6 +29,7 @@ func (f *fakeJSDialogView) Show(_ port.JSDialogRequest, respond func(bool, strin
29	29		f.shown++
30	30		f.respond, f.focus = respond, focus
31	31		}
	32	+	func (f *fakeJSDialogView) Visible() bool { return f.respond != nil }
32	33		func (f *fakeJSDialogView) Hide() {}
33	34		func (f *fakeJSDialogView) RequestFocus() { f.focusRequests++ }
34	35		func (f *fakeJSDialogView) SetOnHidden(fn func(bool)) {

@@ -109,3 +110,39 @@ func TestPaneViewSetActiveFocusesPendingJSDialog(t *testing.T) {
109	110
110	111		assert.Equal(t, 1, fake.focusRequests)
111	112		}
	113	+
	114	+	func TestPaneViewGrabFocusDefersToVisibleJSDialog(t *testing.T) {
	115	+	fake := &fakeJSDialogView{}
	116	+	pv := newJSDialogPaneView(t, fake)
	117	+	webview := mocks.NewMockWidget(t) // GrabFocus on it would fail the test (no expectation)
	118	+	pv.webViewWidget = webview
	119	+	border := mocks.NewMockBoxWidget(t)
	120	+	border.EXPECT().AddCssClass(activePaneClass).Once()
	121	+	pv.borderBox = border
	122	+	require.True(t, pv.ShowJSDialog(port.JSDialogRequest{}, func(bool, string) {}, 1))
	123	+
	124	+	pv.SetActive(true)
	125	+	require.Equal(t, 1, fake.focusRequests)
	126	+	assert.True(t, pv.GrabFocus())
	127	+
	128	+	assert.Equal(t, 2, fake.focusRequests, "dialog focus requested again, WebView not focused")
	129	+	}
	130	+
	131	+	func TestPaneViewGrabFocusFocusesWebViewWithoutDialog(t *testing.T) {
	132	+	fake := &fakeJSDialogView{}
	133	+	pv := newJSDialogPaneView(t, fake)
	134	+	webview := mocks.NewMockWidget(t)
	135	+	webview.EXPECT().GrabFocus().Return(true).Once()
	136	+	pv.webViewWidget = webview
	137	+
	138	+	assert.True(t, pv.GrabFocus())
	139	+	}
	140	+
	141	+	func TestPaneViewShowJSDialogAfterCleanupFails(t *testing.T) {
	142	+	fake := &fakeJSDialogView{}
	143	+	pv := newJSDialogPaneView(t, fake)
	144	+	pv.Cleanup()
	145	+
	146	+	assert.False(t, pv.ShowJSDialog(port.JSDialogRequest{}, func(bool, string) {}, 1))
	147	+	assert.Zero(t, fake.shown)
	148	+	}


@@ -29,6 +29,7 @@ func (f *fakeJSDialogView) Show(_ port.JSDialogRequest, respond func(bool, strin
29	f.shown++	29		f.shown++
30	f.respond, f.focus = respond, focus	30		f.respond, f.focus = respond, focus
31	}	31		}
		32	+	func (f *fakeJSDialogView) Visible() bool { return f.respond != nil }
32	func (f *fakeJSDialogView) Hide() {}	33		func (f *fakeJSDialogView) Hide() {}
33	func (f *fakeJSDialogView) RequestFocus() { f.focusRequests++ }	34		func (f *fakeJSDialogView) RequestFocus() { f.focusRequests++ }
34	func (f *fakeJSDialogView) SetOnHidden(fn func(bool)) {	35		func (f *fakeJSDialogView) SetOnHidden(fn func(bool)) {

@@ -109,3 +110,39 @@ func TestPaneViewSetActiveFocusesPendingJSDialog(t *testing.T) {
109		110
110	assert.Equal(t, 1, fake.focusRequests)	111		assert.Equal(t, 1, fake.focusRequests)
111	}	112		}
		113	+
		114	+	func TestPaneViewGrabFocusDefersToVisibleJSDialog(t *testing.T) {
		115	+	fake := &fakeJSDialogView{}
		116	+	pv := newJSDialogPaneView(t, fake)
		117	+	webview := mocks.NewMockWidget(t) // GrabFocus on it would fail the test (no expectation)
		118	+	pv.webViewWidget = webview
		119	+	border := mocks.NewMockBoxWidget(t)
		120	+	border.EXPECT().AddCssClass(activePaneClass).Once()
		121	+	pv.borderBox = border
		122	+	require.True(t, pv.ShowJSDialog(port.JSDialogRequest{}, func(bool, string) {}, 1))
		123	+
		124	+	pv.SetActive(true)
		125	+	require.Equal(t, 1, fake.focusRequests)
		126	+	assert.True(t, pv.GrabFocus())
		127	+
		128	+	assert.Equal(t, 2, fake.focusRequests, "dialog focus requested again, WebView not focused")
		129	+	}
		130	+
		131	+	func TestPaneViewGrabFocusFocusesWebViewWithoutDialog(t *testing.T) {
		132	+	fake := &fakeJSDialogView{}
		133	+	pv := newJSDialogPaneView(t, fake)
		134	+	webview := mocks.NewMockWidget(t)
		135	+	webview.EXPECT().GrabFocus().Return(true).Once()
		136	+	pv.webViewWidget = webview
		137	+
		138	+	assert.True(t, pv.GrabFocus())
		139	+	}
		140	+
		141	+	func TestPaneViewShowJSDialogAfterCleanupFails(t *testing.T) {
		142	+	fake := &fakeJSDialogView{}
		143	+	pv := newJSDialogPaneView(t, fake)
		144	+	pv.Cleanup()
		145	+
		146	+	assert.False(t, pv.ShowJSDialog(port.JSDialogRequest{}, func(bool, string) {}, 1))
		147	+	assert.Zero(t, fake.shown)
		148	+	}