• Adds new "margin" skin with 11 themes (colibris default + 5 named themes in light/dark)
• Injects Theme dropdown into Settings popup (User Settings + Pad-wide Settings)
• Theme selection persists in localStorage and syncs across pad and lobby
• Vendors colibris component partials for full self-contained skin
• Supports Google Fonts for themed typography stacks (Newsreader, Space Mono, Lora, IBM Plex, VT323)

Theme selector injection and iframe propagation

src/static/skins/margin/pad.js

Lobby theme bootstrap and recent pads UI

src/static/skins/margin/index.js

Theme definitions and component styling

src/static/skins/margin/pad.css

Description

index.css and pad.css hardcode https:// Google Fonts @import URLs instead of using
protocol-independent // URLs. This violates the guideline intended to avoid protocol-specific URL
embedding so the resource works under both HTTP and HTTPS.

Code

src/static/skins/margin/index.css[5]

+@import url("https://fonts.googleapis.com/css2?family=Newsreader:ital,opsz,wght@0,6..72,300..700;1,6..72,300..700&family=Instrument+Serif:ital@0;1&family=Lora:ital,wght@0,400..700;1,400..700&family=Space+Mono:ital,wght@0,400;0,700;1,400;1,700&family=VT323&family=IBM+Plex+Mono:ital,wght@0,400;0,500;0,600;1,400&family=IBM+Plex+Sans:ital,wght@0,400;0,500;0,600;0,700&display=swap");

Evidence

PR Compliance ID 7 requires protocol-independent URLs (//) where applicable, and the cited changes
show @import statements in both src/static/skins/margin/index.css and
src/static/skins/margin/pad.css using https://fonts.googleapis.com/... directly rather than a
protocol-independent form.

src/static/skins/margin/index.css[5-5]
src/static/skins/margin/pad.css[25-25]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Update the Google Fonts `@import` statements in `index.css` and `pad.css` to avoid hardcoding the `https://` protocol and instead use protocol-independent `//` URLs as required by the compliance guideline.
## Issue Context
PR Compliance ID 7 requires protocol-independent URLs (`//`) where applicable; these are fetchable external stylesheet URLs (`fonts.googleapis.com`) that can be expressed in protocol-independent form so they work under both HTTP and HTTPS.
## Fix Focus Areas
- src/static/skins/margin/index.css[5-5]
- src/static/skins/margin/pad.css[25-25]

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

margin/index.js builds recent-pad links via string concatenation with window.location.href,
which can generate invalid URLs (missing trailing slash, query/hash present) and fails for pad names
needing URL encoding. This can make the Recent Pads list navigate to a non-existent page.

Code

src/static/skins/margin/index.js[R84-90]

+ li.className = 'recent-pad';
+ const padPath = `${window.location.href}p/${pad.name}`;
+ const link = document.createElement('a');
+ link.style.textDecoration = 'none';
+
+ link.href = padPath;
+ link.innerText = pad.name;

Evidence

The PR constructs pad URLs with ${window.location.href}p/${pad.name}; this is not robust URL
joining and can yield malformed URLs depending on the lobby URL shape. Elsewhere in the codebase,
navigation URLs are built with new URL(..., window.location.href) to avoid these edge cases.

src/static/skins/margin/index.js[84-90]
src/static/js/pad_editbar.ts[460-468]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`src/static/skins/margin/index.js` constructs recent-pad links by concatenating `window.location.href` with `p/${pad.name}`. This can produce broken URLs (no trailing slash, existing query/hash) and does not URL-encode the pad name.
### Issue Context
Core code uses `new URL(rel, window.location.href)` to safely construct navigation URLs.
### Fix Focus Areas
- src/static/skins/margin/index.js[84-90]
### Suggested change
Use something like:
- `const padPath = new URL(`p/${encodeURIComponent(pad.name)}`, window.location.href).href;`
If Etherpad can be served from a sub-path, keep the base as `window.location.href` (not `origin`) but still use `new URL()` for correct joining.

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

margin/index.js and margin/pad.js call JSON.parse() on the recentPads localStorage value
without error handling, so malformed data (or storage access failures) will throw and abort the
skin’s customStart() execution. This breaks recent-pad rendering on the lobby and recent-pad
history updates from the pad.

Code

src/static/skins/margin/index.js[R45-50]

+ const recentPadListHeading = document.querySelector('[data-l10n-id="index.recentPads"]');
+ const recentPadsFromLocalStorage = localStorage.getItem('recentPads');
+ let recentPadListData = [];
+ if (recentPadsFromLocalStorage != null) {
+ recentPadListData = JSON.parse(recentPadsFromLocalStorage);
+ }

Evidence

Both the lobby and pad skin scripts parse recentPads from localStorage without a try/catch;
JSON.parse() throws on invalid JSON, and localStorage.getItem() can also throw in some
restricted storage modes. Because this is inside window.customStart(), an exception aborts the
rest of that hook for the page.

src/static/skins/margin/index.js[45-50]
src/static/skins/margin/pad.js[151-158]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The margin skin reads and parses `recentPads` from `localStorage` without guarding against malformed JSON or storage access exceptions. A single bad value can throw and stop the skin’s `customStart()`.
### Issue Context
This affects only the skin hook logic (recent pads list rendering on the lobby; recent pads list updates from the pad), but it’s still a user-visible breakage.
### Fix Focus Areas
- src/static/skins/margin/index.js[45-55]
- src/static/skins/margin/pad.js[151-175]
### Suggested change
- Wrap `localStorage.getItem()` + `JSON.parse()` in `try/catch`.
- On error, fall back to `[]`.
- Validate that the parsed value is an array of objects with expected fields before using.
- Wrap `localStorage.setItem()` calls similarly so quota/private-mode failures don’t throw.

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

margin/timeslider.js never applies the saved marginTheme to data-theme, but margin/pad.css
defines theme tokens only under [data-theme="..."] selectors. As a result, the timeslider page
(including history mode’s #history-frame iframe) won’t reflect the selected margin theme.

Code

src/static/skins/margin/timeslider.js[R1-4]

+'use strict';
+
+window.customStart = () => {
+};

Evidence

The margin skin’s theming is driven by data-theme (see the per-theme blocks in pad.css), and the
pad/lobby set it via their skin JS. The timeslider page includes the skin’s pad.css but does not
set data-theme anywhere and the skin’s timeslider.js is empty, so theme selection is not applied
when viewing history (including the in-place history iframe created by pad_mode.ts).

src/static/skins/margin/timeslider.js[1-4]
src/templates/timeslider.html[9-11]
src/templates/timeslider.html[44-50]
src/static/skins/margin/pad.css[49-52]
src/static/skins/margin/pad.css[65-79]
src/static/js/pad_mode.ts[234-244]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The margin theme is selected via `data-theme` and persisted in `localStorage` under `marginTheme`, but `src/static/skins/margin/timeslider.js` does not apply it. Therefore the timeslider page (including history mode’s in-place iframe) does not match the user’s chosen theme.
### Issue Context
- `timeslider.html` loads the skin’s `pad.css` and `timeslider.css`.
- Margin’s theme variables in `pad.css` are defined under `[data-theme="..."]`.
- In history mode, the timeslider page is mounted as an iframe with id `history-frame`.
### Fix Focus Areas
- src/static/skins/margin/timeslider.js[1-4]
### Suggested change
Add the same early theme bootstrap used in `margin/index.js` (preferably with validation like `margin/pad.js`):
- Read `localStorage.getItem('marginTheme')` in a try/catch
- Validate against allowed theme values (or default to `colibris`)
- Set `document.documentElement.setAttribute('data-theme', theme)` as early as possible
Optionally: if `window.parent !== window`, you can also attempt to read the parent document’s `data-theme` as the first choice, falling back to localStorage.

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools