-
-
Notifications
You must be signed in to change notification settings - Fork 48
fix(webui): keep embedded ActivityWatch URLs inside WebView #161
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package net.activitywatch.android.fragments | ||
|
|
||
| import org.junit.Assert.assertFalse | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test | ||
|
|
||
| class WebUIFragmentTest { | ||
| @Test | ||
| fun `treats local embedded server hosts as internal`() { | ||
| // http variants | ||
| assertTrue(isEmbeddedActivityWatchUrl("http://127.0.0.1:5600/#/settings/")) | ||
| assertTrue(isEmbeddedActivityWatchUrl("http://localhost:5600/#/settings/")) | ||
| assertTrue(isEmbeddedActivityWatchUrl("http://[::1]:5600/#/settings/")) | ||
| // https variants (function explicitly allows both schemes) | ||
| assertTrue(isEmbeddedActivityWatchUrl("https://127.0.0.1:5600/")) | ||
| assertTrue(isEmbeddedActivityWatchUrl("https://localhost:5600/")) | ||
| assertTrue(isEmbeddedActivityWatchUrl("https://[::1]:5600/")) | ||
| // no-port case | ||
| assertTrue(isEmbeddedActivityWatchUrl("http://localhost/")) | ||
| } | ||
|
Comment on lines
+9
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 696e8d5 — added https variants for all three loopback hosts plus a no-port case. |
||
|
|
||
| @Test | ||
| fun `treats non-loopback hosts as external`() { | ||
| assertFalse(isEmbeddedActivityWatchUrl("https://activitywatch.net")) | ||
| assertFalse(isEmbeddedActivityWatchUrl("http://192.168.1.10:5600")) | ||
| assertFalse(isEmbeddedActivityWatchUrl("not a url")) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isEmbeddedActivityWatchUrlreturnstruefor every port on loopback, not just 5600. If the in-app WebView ever renders a page from127.0.0.1:5600that links to another local service on a different port (e.g., a dev server at127.0.0.1:3000), that link will silently stay inside the WebView rather than opening in the external browser. The original check usedurl.contains("//localhost:")which implicitly scoped to the scheme+host+port triple; the new helper broadens that. Consider adding a port check (port == 5600, or a small set of known ports) if this widened scope is not intentional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The broader scope is intentional — ActivityWatch can run on non-default ports and we want to keep all loopback traffic in-WebView regardless of port. The original
url.contains("//localhost:")check was actually port-agnostic too (it just required a port to be present). Leaving as-is.