5ead628 fix(review): address copilot PR review feedback · dumber

M internal/domain/url/normalize.go +34 -3


@@ -19,6 +19,23 @@ func expandHome(path string) string {
19	19		return path
20	20		}
21	21
	22	+	// looksLikeFilePath returns true if the input looks like a filesystem path.
	23	+	// This includes absolute paths (/path), relative paths (./path, ../path),
	24	+	// and home-relative paths (~/path).
	25	+	func looksLikeFilePath(input string) bool {
	26	+	switch {
	27	+	case strings.HasPrefix(input, "/"):
	28	+	return true
	29	+	case strings.HasPrefix(input, "./"):
	30	+	return true
	31	+	case strings.HasPrefix(input, "../"):
	32	+	return true
	33	+	case strings.HasPrefix(input, "~/"):
	34	+	return true
	35	+	}
	36	+	return false
	37	+	}
	38	+
22	39		// Normalize adds https:// prefix if missing for URL-like inputs.
23	40		// Returns the input unchanged if it already has a scheme or doesn't look like a URL.
24	41		// If the input is an existing local file, returns a file:// URL.

@@ -41,9 +58,23 @@ func Normalize(input string) string {
41	58		return input
42	59		}
43	60
44		-	// Check if input is an existing file path
45		-	expanded := expandHome(input)
46		-	absPath, err := filepath.Abs(expanded)
	61	+	// Check if input looks like a file path
	62	+	if looksLikeFilePath(input) {
	63	+	expanded := expandHome(input)
	64	+	absPath, err := filepath.Abs(expanded)
	65	+	if err == nil {
	66	+	if _, statErr := os.Stat(absPath); statErr == nil {
	67	+	// File exists, convert to file:// URL
	68	+	return "file://" + absPath
	69	+	}
	70	+	}
	71	+	// Path-like input but file doesn't exist - return unchanged
	72	+	// Don't treat as URL to avoid malformed URLs like https:///path
	73	+	return input
	74	+	}
	75	+
	76	+	// Check if input is an existing file in current directory (relative without ./ prefix)
	77	+	absPath, err := filepath.Abs(input)
47	78		if err == nil {
48	79		if _, statErr := os.Stat(absPath); statErr == nil {
49	80		return "file://" + absPath


@@ -19,6 +19,23 @@ func expandHome(path string) string {
19		return path	19		return path
20		}	20		}
21			21
			22	+	// looksLikeFilePath returns true if the input looks like a filesystem path.
			23	+	// This includes absolute paths (/path), relative paths (./path, ../path),
			24	+	// and home-relative paths (~/path).
			25	+	func looksLikeFilePath(input string) bool {
			26	+	switch {
			27	+	case strings.HasPrefix(input, "/"):
			28	+	return true
			29	+	case strings.HasPrefix(input, "./"):
			30	+	return true
			31	+	case strings.HasPrefix(input, "../"):
			32	+	return true
			33	+	case strings.HasPrefix(input, "~/"):
			34	+	return true
			35	+	}
			36	+	return false
			37	+	}
			38	+
22		// Normalize adds https:// prefix if missing for URL-like inputs.	39		// Normalize adds https:// prefix if missing for URL-like inputs.
23		// Returns the input unchanged if it already has a scheme or doesn't look like a URL.	40		// Returns the input unchanged if it already has a scheme or doesn't look like a URL.
24		// If the input is an existing local file, returns a file:// URL.	41		// If the input is an existing local file, returns a file:// URL.

@@ -41,9 +58,23 @@ func Normalize(input string) string {
41		return input	58		return input
42		}	59		}
43			60
44	-	// Check if input is an existing file path	61	+	// Check if input looks like a file path
45	-	expanded := expandHome(input)	62	+	if looksLikeFilePath(input) {
46	-	absPath, err := filepath.Abs(expanded)	63	+	expanded := expandHome(input)
			64	+	absPath, err := filepath.Abs(expanded)
			65	+	if err == nil {
			66	+	if _, statErr := os.Stat(absPath); statErr == nil {
			67	+	// File exists, convert to file:// URL
			68	+	return "file://" + absPath
			69	+	}
			70	+	}
			71	+	// Path-like input but file doesn't exist - return unchanged
			72	+	// Don't treat as URL to avoid malformed URLs like https:///path
			73	+	return input
			74	+	}
			75	+
			76	+	// Check if input is an existing file in current directory (relative without ./ prefix)
			77	+	absPath, err := filepath.Abs(input)
47		if err == nil {	78		if err == nil {
48		if _, statErr := os.Stat(absPath); statErr == nil {	79		if _, statErr := os.Stat(absPath); statErr == nil {
49		return "file://" + absPath	80		return "file://" + absPath

M internal/domain/url/normalize_test.go +12 -2


@@ -108,9 +108,19 @@ func TestNormalize_LocalFiles(t *testing.T) {
108	108		want: "file://" + tmpDir,
109	109		},
110	110		{
111		-	name: "non-existent file treated as URL",
	111	+	name: "non-existent absolute path returned unchanged",
112	112		input: "/nonexistent/file.html",
113		-	want: "https:///nonexistent/file.html",
	113	+	want: "/nonexistent/file.html",
	114	+	},
	115	+	{
	116	+	name: "non-existent relative path with dot prefix returned unchanged",
	117	+	input: "./nonexistent/file.html",
	118	+	want: "./nonexistent/file.html",
	119	+	},
	120	+	{
	121	+	name: "non-existent home path returned unchanged",
	122	+	input: "~/nonexistent/file.html",
	123	+	want: "~/nonexistent/file.html",
114	124		},
115	125		}
116	126


@@ -108,9 +108,19 @@ func TestNormalize_LocalFiles(t *testing.T) {
108		want: "file://" + tmpDir,	108		want: "file://" + tmpDir,
109		},	109		},
110		{	110		{
111	-	name: "non-existent file treated as URL",	111	+	name: "non-existent absolute path returned unchanged",
112		input: "/nonexistent/file.html",	112		input: "/nonexistent/file.html",
113	-	want: "https:///nonexistent/file.html",	113	+	want: "/nonexistent/file.html",
			114	+	},
			115	+	{
			116	+	name: "non-existent relative path with dot prefix returned unchanged",
			117	+	input: "./nonexistent/file.html",
			118	+	want: "./nonexistent/file.html",
			119	+	},
			120	+	{
			121	+	name: "non-existent home path returned unchanged",
			122	+	input: "~/nonexistent/file.html",
			123	+	want: "~/nonexistent/file.html",
114		},	124		},
115		}	125		}
116			126

M internal/ui/component/link_status.go +16 -2


@@ -3,7 +3,6 @@ package component
3	3
4	4		import (
5	5		"sync"
6		-	"time"
7	6
8	7		"github.com/jwijenbergh/puregotk/v4/glib"
9	8		"github.com/jwijenbergh/puregotk/v4/gtk"

@@ -102,7 +101,7 @@ func (l *LinkStatusOverlay) Show(uri string) {
102	101		return false // Don't repeat
103	102		})
104	103
105		-	l.showTimer = glib.TimeoutAdd(uint(linkStatusShowDelayMs/time.Millisecond), &cb, 0)
	104	+	l.showTimer = glib.TimeoutAdd(linkStatusShowDelayMs, &cb, 0)
106	105		}
107	106
108	107		// Hide manually hides the link status overlay.

@@ -138,3 +137,18 @@ func (l *LinkStatusOverlay) IsVisible() bool {
138	137		defer l.mu.Unlock()
139	138		return l.visible
140	139		}
	140	+
	141	+	// Cleanup cancels any pending timers and clears state.
	142	+	// Must be called before removing the overlay from the UI.
	143	+	func (l *LinkStatusOverlay) Cleanup() {
	144	+	l.mu.Lock()
	145	+	defer l.mu.Unlock()
	146	+
	147	+	// Cancel pending show timer to prevent use-after-free
	148	+	if l.showTimer != 0 {
	149	+	glib.SourceRemove(l.showTimer)
	150	+	l.showTimer = 0
	151	+	}
	152	+	l.pendingURI = ""
	153	+	l.visible = false
	154	+	}

 import (
 	"sync"
-	"time"
 	"github.com/jwijenbergh/puregotk/v4/glib"
 	"github.com/jwijenbergh/puregotk/v4/gtk"
 		return false // Don't repeat
 	})
-	l.showTimer = glib.TimeoutAdd(uint(linkStatusShowDelayMs/time.Millisecond), &cb, 0)
+	l.showTimer = glib.TimeoutAdd(linkStatusShowDelayMs, &cb, 0)
 }
 // Hide manually hides the link status overlay.
 	defer l.mu.Unlock()
 	return l.visible
 }
+// Cleanup cancels any pending timers and clears state.
+// Must be called before removing the overlay from the UI.
+func (l *LinkStatusOverlay) Cleanup() {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	// Cancel pending show timer to prevent use-after-free
+	if l.showTimer != 0 {
+		glib.SourceRemove(l.showTimer)
+		l.showTimer = 0
+	}
+	l.pendingURI = ""
+	l.visible = false
+}

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


@@ -421,6 +421,7 @@ func (pv *PaneView) Cleanup() {
421	421		pv.toaster = nil
422	422		}
423	423		if pv.linkStatus != nil {
	424	+	pv.linkStatus.Cleanup() // Cancel pending timers before removal
424	425		pv.overlay.RemoveOverlay(pv.linkStatus.Widget())
425	426		pv.linkStatus = nil
426	427		}


@@ -421,6 +421,7 @@ func (pv *PaneView) Cleanup() {
421	pv.toaster = nil	421		pv.toaster = nil
422	}	422		}
423	if pv.linkStatus != nil {	423		if pv.linkStatus != nil {
		424	+	pv.linkStatus.Cleanup() // Cancel pending timers before removal
424	pv.overlay.RemoveOverlay(pv.linkStatus.Widget())	425		pv.overlay.RemoveOverlay(pv.linkStatus.Widget())
425	pv.linkStatus = nil	426		pv.linkStatus = nil
426	}	427		}

M internal/ui/coordinator/content.go +0 -5


@@ -190,11 +190,6 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa
190	190		return c.handleLinkMiddleClick(ctx, paneID, uri)
191	191		}
192	192
193		-	// Set up link hover callback for status overlay
194		-	wv.OnLinkHover = func(uri string) {
195		-	c.onLinkHover(paneID, uri)
196		-	}
197		-
198	193		// Set up fullscreen handlers for idle inhibition
199	194		c.setupIdleInhibitionHandlers(ctx, paneID, wv)
200	195


@@ -190,11 +190,6 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa
190		return c.handleLinkMiddleClick(ctx, paneID, uri)	190	return c.handleLinkMiddleClick(ctx, paneID, uri)
191		}	191	}
192			192
193	-	// Set up link hover callback for status overlay
194	-	wv.OnLinkHover = func(uri string) {
195	-	c.onLinkHover(paneID, uri)
196	-	}
197	-
198		// Set up fullscreen handlers for idle inhibition	193	// Set up fullscreen handlers for idle inhibition
199		c.setupIdleInhibitionHandlers(ctx, paneID, wv)	194	c.setupIdleInhibitionHandlers(ctx, paneID, wv)
200			195