fix: correctly parse the path in getloc #4940

K3das · 2025-09-16T23:43:51Z

If you put WLED behind a reverse proxy, getLoc will ignore the reverse proxy if it is only one subpath (for example, if I had WLED on /lights/, getLoc would not find the subpath, and settings pages would not load).

Instead of blindly trimming the last path, this change has getLoc trim anything after /settings/.

Summary by CodeRabbit

Bug Fixes
- More reliable base URL detection for apps served behind reverse proxies or subpaths.
- Correct handling of URLs with trailing "settings" segments and file-like segments, preventing broken links or missing assets.
- Works correctly for single-segment paths and instances without a "settings" section.
Refactor
- Simplified and more robust path parsing to improve navigation and asset loading across varied deployments.

coderabbitai · 2025-09-16T23:43:58Z

Walkthrough

Rewrote getLoc() path normalization in wled00/data/common.js to derive the base URL by locating the last "settings" segment or dropping a trailing file-like segment; locproto/loc/locip are now set whenever any path segments remain, changing base URL behavior for various path patterns.

Changes

Cohort / File(s)	Summary
Path normalization logic `wled00/data/common.js`	Replaced pop-based segment removal with index-based trimming: find last `"settings"` segment and slice path up to it if at or just before the end; otherwise drop a trailing file-like segment (contains a dot). Set `locproto`/`loc`/`locip` whenever any path segments remain. Adjusts base URL derivation for paths with or without `"settings"`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "fix: correctly parse the path in getloc" clearly and concisely summarizes the primary change—fixing getLoc's path parsing—matching the PR objective to handle reverse-proxy subpaths (e.g., trimming after "/settings/") and is specific enough for teammates scanning history.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50ea832 and 0949dd1.

📒 Files selected for processing (1)

wled00/data/common.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

wled00/data/common.js

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

wled00/data/common.js (1)

86-102: Critical: base-path derivation now includes filenames and root “/” empties, breaking non-settings pages

When URL has no “settings” segment (e.g., “/index.htm”), paths isn’t trimmed and locip becomes host/index.htm, so getURL("/settings") yields “…/index.htm/settings”. Root “/” also sets loc due to [""] length. Normalize segments, drop trailing files, then trim “settings”. Patch below.

-		let path = l.pathname;
-		let paths = path.slice(1,path.endsWith('/')?-1:undefined).split("/");
-
-		// remove settings paths to get the base url
-		const settingsIndex = paths.lastIndexOf("settings");
-		const pathLength = paths.length;
-		if (
-			settingsIndex != -1 &&
-			(settingsIndex === pathLength - 1 || settingsIndex === pathLength - 2)
-		) {
-			paths = paths.slice(0, settingsIndex);
-		}
-
-		if (paths.length > 0) {
+		let path = l.pathname;
+		let paths = path.slice(1,path.endsWith('/')?-1:undefined).split("/");
+		// normalize: drop empties from leading/trailing slashes
+		paths = paths.filter(Boolean);
+		// remove settings paths to get the base url
+		const settingsIndex = paths.lastIndexOf("settings");
+		const endsWithSettings = settingsIndex !== -1 && (settingsIndex === paths.length - 1 || settingsIndex === paths.length - 2);
+		if (endsWithSettings) {
+			paths = paths.slice(0, settingsIndex);
+		} else if (paths.length && paths[paths.length - 1].includes(".")) {
+			// drop trailing file like index.htm
+			paths.pop();
+		}
+
+		if (paths.length > 0) {
 			locproto = l.protocol;
 			loc = true;
 			locip = l.hostname + (l.port ? ":" + l.port : "") + "/" + paths.join('/');
 		}

🧹 Nitpick comments (1)

wled00/data/common.js (1)

92-93: Use strict inequality for index checks

Prefer !== -1 over != -1 for lastIndexOf results. The patch above includes this.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76cb2e9 and 50ea832.

📒 Files selected for processing (1)

wled00/data/common.js (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

wled00/data/**/*.{htm,html,css,js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use tabs for indentation in web UI files (.htm/.html/.css/.js) under wled00/data

Files:

wled00/data/common.js

🧬 Code graph analysis (1)

wled00/data/common.js (1)

wled00/data/index.js (1)

paths (234-234)

🔇 Additional comments (1)

wled00/data/common.js (1)

88-96: Use tabs for indentation in wled00/data/common.js (lines 88–96)
Script returned no output; verify these lines start with tabs (not spaces) to comply with wled00/data/* formatting rules.

wled00/data/common.js

blazoncek · 2025-09-18T18:31:08Z

I've come up with this:

let paths = l.pathname.split("/").slice(1); // first is always empty
let settingsIndex = paths.lastIndexOf("settings");
paths = paths.slice(0,settingsIndex); // if we don't have "settings", remove last entry (empty / or file)

Reasoning:

main UI's path is always at / (reverse proxy: /path/to/UI/) and is terminated with / (/index.htm is invalid address)
several auxiliary pages start as /page.htm
any setting page will start with /settings

So, possible landing URL paths (that can call common.js and invoke getLoc()) are:

/
/settings (or /settings/)
/settings/subpage (or /settings/subpage/)
/file.htm
Which translate to:
/path/to/UI/ requires terminating /
/path/to/UI/settings (or /path/to/UI/settings/)
/path/to/UI/settings/subpage (or /path/to/UI/settings/subpage/)
/path/to/UI/file.htm

K3das · 2025-09-18T18:42:08Z

Oooh, that's a really neat way to do it! I don't see why it wouldn't work.

Co-authored-by: Blaž Kristan <blaz@kristan-sp.si>

blazoncek

... and you are good.

wled00/data/common.js

blazoncek · 2025-09-18T18:59:43Z

Please do test as I currently do not have reverse proxy set up for WLED.

K3das · 2025-09-18T19:13:15Z

works on my WLED instance where I was having issues :)

K3das added 4 commits September 16, 2025 23:18

fix: correctly parse the path in getloc

a6068fe

fix: properly trim settings-related paths

414eb45

fix: check that settings is in the path

888877e

fix: formatting

50ea832

coderabbitai bot reviewed Sep 16, 2025

View reviewed changes

wled00/data/common.js Show resolved Hide resolved

fix: cover trailing file cases and coderabbit changes

0949dd1

blazoncek reviewed Sep 17, 2025

View reviewed changes

wled00/data/common.js Show resolved Hide resolved

fix: simplify slice logic

aea1a0f

fix: cleaner path logic

e948f68

Co-authored-by: Blaž Kristan <blaz@kristan-sp.si>

K3das requested a review from blazoncek September 18, 2025 18:44

blazoncek approved these changes Sep 18, 2025

View reviewed changes

wled00/data/common.js Outdated Show resolved Hide resolved

fix: trailing spaces

b4e1394

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: correctly parse the path in getloc #4940

fix: correctly parse the path in getloc #4940

Uh oh!

K3das commented Sep 16, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 16, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

blazoncek commented Sep 18, 2025

Uh oh!

K3das commented Sep 18, 2025

Uh oh!

blazoncek left a comment

Uh oh!

Uh oh!

blazoncek commented Sep 18, 2025

Uh oh!

K3das commented Sep 18, 2025

Uh oh!

Uh oh!

Uh oh!

fix: correctly parse the path in getloc #4940

Are you sure you want to change the base?

fix: correctly parse the path in getloc #4940

Uh oh!

Conversation

K3das commented Sep 16, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

blazoncek commented Sep 18, 2025

Uh oh!

K3das commented Sep 18, 2025

Uh oh!

blazoncek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blazoncek commented Sep 18, 2025

Uh oh!

K3das commented Sep 18, 2025

Uh oh!

Uh oh!

K3das commented Sep 16, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 16, 2025 •

edited

Loading