Fix: stop the importer losing sections and refusing valid configs

CHANGELOG.md

@@ -63,6 +63,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The password and Unsplash key status labels are translated instead of always
   reading in English.
 
+- Escape and the Tab trap keep working in a folder or prompt overlay after
+  clicking text inside it.
+
 ### Security
 
 - A stored credential is only reused for the request it was saved for.

ui/admin/index.html

@@ -285,6 +285,6 @@ <h1 class="sec-title" id="ev-title">General</h1>
   </div></div>
 </div>
 
-<script type="module" src="/js/admin.js?v=78a1f3fe"></script>
+<script type="module" src="/js/admin.js?v=8c5189ca"></script>
 </body>
 </html>

ui/index.html

@@ -54,5 +54,5 @@ <h1 class="visually-hidden">Stackyard</h1>
     </div>
   </div>
 </div>
-<script type="module" src="/js/dashboard.js?v=dd2976fb"></script>
+<script type="module" src="/js/dashboard.js?v=4693af36"></script>
 </body></html>

ui/js/admin.js

@@ -13,9 +13,9 @@ import { loadSettings, showBgFields } from '/js/admin-settings.js?v=f189373f';
 import { canJoinFolder, applyDrop } from '/js/admin-drag-logic.js?v=53aeaa55';
 /* openModal is the item editor in this file, so the shared dialog comes in
    under a different name. */
-import { openModal as openDialog, confirmModal, promptModal } from '/js/modal.js?v=10d34989';
-import { parseYaml, YamlLiteError } from '/js/yaml-lite.js?v=a3f8f22e';
-import { detectSource, convert, SKIP, NOTE } from '/js/import-foreign.js?v=e0db2fb3';
+import { openModal as openDialog, confirmModal, promptModal } from '/js/modal.js?v=684b74e0';
+import { parseYaml, YamlLiteError } from '/js/yaml-lite.js?v=14aa75ec';
+import { detectSource, convert, SKIP, NOTE } from '/js/import-foreign.js?v=cf38ec0b';
 
 /* Admin UI: Stackyard Dashboard */
 

ui/js/dashboard.js

@@ -22,15 +22,15 @@ import {
   qa,
   setUserText,
 } from '/js/utils.js?v=84d58686';
-import { initSpotlight } from '/js/spotlight.js?v=a5ba7f75';
+import { initSpotlight } from '/js/spotlight.js?v=7aa670dc';
 import { html, setHtml, raw } from '/js/html.js?v=ccec347c';
 import { initI18n, t, currentLang } from '/js/i18n.js?v=133a7aac';
 import { pwStrength, passwordMismatch } from '/js/password-strength.js?v=dab9978e';
 import { sanitizeItemLinks } from '/js/link-url.js?v=19038560';
-import { initUI, mkFolder, openFolderDesktop, openFolderMobile, buildMobile } from '/js/ui.js?v=3d60a9bc';
+import { initUI, mkFolder, openFolderDesktop, openFolderMobile, buildMobile } from '/js/ui.js?v=1515288b';
 import { badgeSignature, computeBadgeVisual } from '/js/badge-logic.js?v=d278c683';
 import { configChanged, landingAfterSetup } from '/js/dashboard-logic.js?v=640430ba';
-import { trapFocus } from '/js/dialog.js?v=4ff94595';
+import { trapFocus } from '/js/dialog.js?v=b3841546';
 
 const MOB = innerWidth <= 768 || /iPhone|iPod|Android/i.test(navigator.userAgent);
 

ui/js/dialog.js

@@ -89,7 +89,7 @@ export function trapFocus(root, opts = {}) {
   function release() {
     if (released) return;
     released = true;
-    root.removeEventListener('keydown', /** @type {EventListener} */ (onKeydown));
+    document.removeEventListener('keydown', /** @type {EventListener} */ (onKeydown), true);
     /* Without this, closing leaves focus on nothing and the next Tab starts from
        the top of the page. Skipped if the element has left the document. */
     if (restoreTo && restoreTo.focus && restoreTo.isConnected) {
@@ -101,7 +101,12 @@ export function trapFocus(root, opts = {}) {
     }
   }
 
-  root.addEventListener('keydown', /** @type {EventListener} */ (onKeydown));
+  /* On the document rather than on root, because a dialog holds text that is not
+     focusable: clicking a line of it moves focus to the body, and a listener
+     inside root would then never see the key. Escape and the Tab wrap have to
+     work from wherever the click left focus, which is the case wrapTab's
+     out-of-root branch already covers. */
+  document.addEventListener('keydown', /** @type {EventListener} */ (onKeydown), true);
 
   const target = initialFocus || focusableWithin(root)[0];
   if (target && target.focus) {

ui/js/import-foreign.js

@@ -131,7 +131,7 @@ export function convertIcon(raw) {
     }
     return host === 'cdn.jsdelivr.net' ? { iconUrl: s, dropped: false } : { iconUrl: '', dropped: true };
   }
-  if (s.startsWith('/') || s.includes('/')) return { iconUrl: '', dropped: true };
+  if (s.includes('/')) return { iconUrl: '', dropped: true };
   /* selfh.st and homelab-svg-assets prefixes. Both name the same service the
      dashboard-icons catalogue does, so the remainder is a usable slug. */
   const body = /^(sh|hl)-/i.test(s) ? s.slice(3) : s;
@@ -148,9 +148,11 @@ export function convertIcon(raw) {
 
 /** Collector for one conversion run. Keeps ids unique across every file in the
     batch, including against the items already on the dashboard.
+    A Set given by the caller is used as it is rather than copied: the ids of one
+    file have to be visible while the next one in the batch is converted.
     @param {Iterable<string>} takenIds */
 function collector(takenIds) {
-  const taken = new Set(takenIds || []);
+  const taken = takenIds instanceof Set ? takenIds : new Set(takenIds || []);
   return {
     taken,
     /** @type {any[]} */ items: [],
@@ -242,7 +244,7 @@ export function convertHomepageServices(doc, takenIds = []) {
   const col = collector(takenIds);
   if (!Array.isArray(doc)) return result(col);
 
-  const walk = (entries, groupLabel, depth) => {
+  const walk = (entries, groupLabel) => {
     const at = col.items.length;
     const children = [];
     for (const entry of Array.isArray(entries) ? entries : []) {
@@ -262,7 +264,7 @@ export function convertHomepageServices(doc, takenIds = []) {
         /* The child's own name, not the compound one: the preview prints
            "group / name", so passing the compound name repeats the parent. */
         col.note(NOTE.GROUP_FLATTENED, name, groupLabel, nested);
-        walk(value, nested, depth + 1);
+        walk(value, nested);
         continue;
       }
       if (!isMap(value)) {
@@ -295,8 +297,14 @@ export function convertHomepageServices(doc, takenIds = []) {
 
   for (const group of doc) {
     const g = soleEntry(group);
-    if (!g || !Array.isArray(g[1])) continue;
-    walk(g[1], g[0], 0);
+    if (!g || !Array.isArray(g[1])) {
+      /* A top-level group the file shapes differently from the rest. detectSource
+         settles the format on the first entry that matches, so one of these does
+         not stop the import, and passing over it loses the whole group. */
+      col.skip(SKIP.UNREADABLE, g ? g[0] : '', '');
+      continue;
+    }
+    walk(g[1], g[0]);
   }
   return result(col);
 }
@@ -308,7 +316,10 @@ export function convertHomepageBookmarks(doc, takenIds = []) {
 
   for (const group of doc) {
     const g = soleEntry(group);
-    if (!g || !Array.isArray(g[1])) continue;
+    if (!g || !Array.isArray(g[1])) {
+      col.skip(SKIP.UNREADABLE, g ? g[0] : '', '');
+      continue;
+    }
     const [groupLabel, entries] = g;
     const at = col.items.length;
     const children = [];
@@ -371,8 +382,6 @@ export function convertDashy(doc, takenIds = [], untitledFolder = 'Imported') {
       }
       const title = text(raw.title);
       const icon = convertIcon(raw.icon);
-      /* localUrl is the address on the other network. Importing it would give
-         half the tiles a link that only works from somewhere else. */
       const href = str(raw.url);
       const check = raw.statusCheck === true;
       /* Dashy's per-item switch for a health check against a self-signed
@@ -390,15 +399,22 @@ export function convertDashy(doc, takenIds = [], untitledFolder = 'Imported') {
       children.push(app.id);
       if (icon.dropped) col.note(NOTE.ICON_DROPPED, title, groupLabel, str(raw.icon));
       if (viaSubItem) col.note(NOTE.SUBITEMS_FLATTENED, title, groupLabel);
-      /* A second address for the same service, reachable only from the other
-         network. Reported rather than dropped in silence, because the tile
-         keeps whichever of the two the source called the primary one. */
+      /* localUrl is a second address for the same service, reachable only from
+         the other network, so importing it would give half the tiles a link that
+         works from somewhere else. Reported rather than dropped in silence,
+         because the tile keeps whichever of the two the source called primary. */
       if (str(raw.localUrl)) col.note(NOTE.LOCAL_URL_DROPPED, title, groupLabel, str(raw.localUrl));
       noteIgnored(col, raw, title, groupLabel);
       for (const sub of Array.isArray(raw.subItems) ? raw.subItems : []) addDashyItem(sub, true);
     };
 
-    for (const item of Array.isArray(section.items) ? section.items : []) addDashyItem(item, false);
+    if (Array.isArray(section.items)) for (const item of section.items) addDashyItem(item, false);
+    else if (!Array.isArray(section.widgets)) {
+      /* A section shaped like one but carrying nothing readable. Without this it
+         contributes no items, no skips and no notes, and the preview shows a
+         file smaller than the one that was chosen with nothing saying why. */
+      col.skip(SKIP.UNREADABLE, groupLabel, '');
+    }
     /* A section widget has no link of its own to keep, unlike a Homepage
        service widget, so there is nothing to import in its place. */
     if (Array.isArray(section.widgets) && section.widgets.length)

ui/js/modal.js

@@ -9,7 +9,7 @@
 
    Structure only. Callers fill the body with their own nodes. */
 
-import { trapFocus } from '/js/dialog.js?v=4ff94595';
+import { trapFocus } from '/js/dialog.js?v=b3841546';
 
 /** Open a modal and return its parts.
 
@@ -57,8 +57,15 @@ export function openModal({ title, className, onClose }) {
     if (onClose) onClose();
   };
 
+  /* Both ends of the click, or a text selection that starts inside the dialog
+     and is released past its edge dismisses it, discarding whatever the person
+     was reading closely enough to select. */
+  let downOnBackdrop = false;
+  ov.onmousedown = e => {
+    downOnBackdrop = e.target === ov;
+  };
   ov.onclick = e => {
-    if (e.target === ov) close();
+    if (e.target === ov && downOnBackdrop) close();
   };
 
   const addAction = (label, cls, onAct) => {
@@ -79,6 +86,9 @@ export function openModal({ title, className, onClose }) {
   document.body.appendChild(ov);
 
   const focus = initial => {
+    /* Arming twice is a caller re-rendering the body. The previous trap has to
+       go, or its listener outlives the dialog and its restore target is lost. */
+    release();
     release = trapFocus(box, { onClose: close, initialFocus: initial });
   };
 

ui/js/spotlight.js

@@ -1,5 +1,5 @@
 import { mk, clr, el, inp as inpById, q, qa, setUserText } from '/js/utils.js?v=84d58686';
-import { wrapTab } from '/js/dialog.js?v=4ff94595';
+import { wrapTab } from '/js/dialog.js?v=b3841546';
 import { t } from '/js/i18n.js?v=133a7aac';
 
 /* Attached to the window so a re-open can undo the previous one. Not a

ui/js/ui.js

@@ -12,7 +12,7 @@ import {
   setUserText,
 } from '/js/utils.js?v=84d58686';
 import { t, currentLang } from '/js/i18n.js?v=133a7aac';
-import { trapFocus } from '/js/dialog.js?v=4ff94595';
+import { trapFocus } from '/js/dialog.js?v=b3841546';
 import { mobileMetrics } from '/js/mobile-metrics.js?v=b32c23b3';
 
 let _state = null;

ui/js/yaml-lite.js

@@ -34,14 +34,19 @@ export class YamlLiteError extends Error {
    "url: http://host:8080" splits once and keeps the port. */
 const KEY_RE = /^(?:(?:"((?:[^"\\]|\\.)*)")|(?:'((?:[^']|'')*)')|([^:#]+?))\s*:(?:\s+(.*))?$/;
 
+/** The escapes a double-quoted form decodes. Shared so a key and a value spell
+    the same text the same way.
+    @param {string} s */
+const unescapeDouble = s => s.replace(/\\(["\\/nrt])/g, (_, c) => ({ n: '\n', r: '\r', t: '\t' })[c] || c);
+
 /** @param {string} raw @param {number} line */
 function scalar(raw, line) {
   const s = raw.trim();
   if (s === '') return '';
   if (s[0] === '"') {
     const m = /^"((?:[^"\\]|\\.)*)"\s*$/.exec(s);
     if (!m) throw new YamlLiteError('unterminated double-quoted value', line);
-    return m[1].replace(/\\(["\\/nrt])/g, (_, c) => ({ n: '\n', r: '\r', t: '\t' })[c] || c);
+    return unescapeDouble(m[1]);
   }
   if (s[0] === "'") {
     const m = /^'((?:[^']|'')*)'\s*$/.exec(s);
@@ -81,6 +86,7 @@ const BLOCK_RE = /^(.*?):\s*([|>])([-+]?)(\d*)\s*$/;
     @param {string[]} raw the block's lines, already stripped of its indentation
     @param {string} style either "|" or ">" @param {string} chomp */
 function foldBlock(raw, style, chomp) {
+  if (!raw.length) return '';
   let body;
   if (style === '|') body = raw.join('\n');
   else {
@@ -93,6 +99,8 @@ function foldBlock(raw, style, chomp) {
     }
   }
   if (chomp === '-') return body.replace(/\n+$/, '');
+  /* "keep" means every trailing newline the block ended with, so the blank lines
+     have to still be here rather than trimmed off before the fold. */
   if (chomp === '+') return body + '\n';
   return body.replace(/\n+$/, '') + (body.length ? '\n' : '');
 }
@@ -106,13 +114,16 @@ function scan(text) {
      becomes part of the first key and the document reads as malformed. */
   if (text.charCodeAt(0) === 0xfeff) text = text.slice(1);
   const rows = text.split(/\r\n|\r|\n/);
+  /* The empty string after a final newline is not a line of the file. It only
+     shows where a block scalar counts the blank lines it ends with. */
+  if (rows.length > 1 && rows[rows.length - 1] === '') rows.pop();
 
   for (let i = 0; i < rows.length; i++) {
     const raw = rows[i];
     const line = i + 1;
     if (/^\s*$/.test(raw)) continue;
     if (/^\s*#/.test(raw)) continue;
-    if (/^\t/.test(raw) || /^ *\t/.test(raw)) throw new YamlLiteError('tab indentation is not supported', line);
+    if (/^ *\t/.test(raw)) throw new YamlLiteError('tab indentation is not supported', line);
     const trimmed = raw.trim();
     if (trimmed === '---') {
       /* One leading marker is ordinary. A second means a multi-document file,
@@ -121,10 +132,11 @@ function scan(text) {
       continue;
     }
     if (trimmed === '...') throw new YamlLiteError('multi-document files are not supported', line);
-    /* Before the anchor check: a merge key is written with an alias, and naming
-       the alias would send someone looking for the wrong thing. */
+    /* Named before the general anchor refusal, which a merge key would also
+       trip, because naming the alias sends someone looking for the wrong thing.
+       Anchors themselves are refused where a node begins rather than here: a
+       value is free text, and `description: The *arr stack` is not an alias. */
     if (/^<<\s*:/.test(trimmed)) throw new YamlLiteError('merge keys are not supported', line);
-    if (/(^|\s)[&*][^\s]/.test(trimmed)) throw new YamlLiteError('anchors and aliases are not supported', line);
 
     const indent = raw.length - raw.trimStart().length;
     const bm = BLOCK_RE.exec(trimmed);
@@ -148,7 +160,6 @@ function scan(text) {
         if (ri < base) break;
         body.push(r.slice(base));
       }
-      while (body.length && body[body.length - 1] === '') body.pop();
       out.push({ indent, text: bm[1].trim() + ':', line, block: foldBlock(body, bm[2], bm[3]) });
       i = j - 1;
       continue;
@@ -165,6 +176,13 @@ function block(lines, cur, indent) {
   return first.text.startsWith('- ') || first.text === '-' ? sequence(lines, cur, indent) : mapping(lines, cur, indent);
 }
 
+/** Refuse an anchor or alias where a node begins. Only the start of a node is
+    checked, so the same characters inside a value stay ordinary text.
+    @param {string} text @param {number} line */
+function refuseAnchor(text, line) {
+  if (/^[&*]\S/.test(text)) throw new YamlLiteError('anchors and aliases are not supported', line);
+}
+
 /** @param {Line[]} lines @param {{ pos: number }} cur @param {number} indent */
 function sequence(lines, cur, indent) {
   const out = [];
@@ -173,20 +191,32 @@ function sequence(lines, cur, indent) {
     if (l.indent < indent) break;
     if (l.indent > indent) throw new YamlLiteError('unexpected indentation', l.line);
     if (!(l.text.startsWith('- ') || l.text === '-')) break;
-    const rest = l.text === '-' ? '' : l.text.slice(2).trim();
+    const after = l.text.slice(1);
+    const rest = after.trim();
     cur.pos++;
     if (rest === '') {
       /* "-" alone: the element is the block indented under it. */
       if (cur.pos < lines.length && lines[cur.pos].indent > indent) out.push(block(lines, cur, lines[cur.pos].indent));
       else out.push(null);
       continue;
     }
+    refuseAnchor(rest, l.line);
+    if (rest.startsWith('- ') || rest === '-')
+      throw new YamlLiteError('a sequence inside a sequence line is not supported', l.line);
+    /* Ahead of the key match, or a flow mapping's own brace and first key read
+       as a key line and the element parses into something that was never in the
+       file. scalar refuses it, and allows the empty pair. */
+    if (rest[0] === '{' || rest[0] === '[') {
+      out.push(scalar(rest, l.line));
+      continue;
+    }
     const m = KEY_RE.exec(rest);
     if (m) {
       /* The compound "- key: value" form. The element is a mapping whose first
-         key sits on the dash line, so its own indentation is the column the
-         key text starts at, not the dash. */
-      const inner = indent + 2;
+         key sits on the dash line, so its own indentation is the column the key
+         text starts at, not a fixed offset from the dash: "-  key" is as valid
+         as "- key" and its continuation lines line up with the key. */
+      const inner = indent + 1 + (after.length - after.trimStart().length);
       const map = Object.create(null);
       assign(map, m, l.line, lines, cur, inner, l);
       while (cur.pos < lines.length && lines[cur.pos].indent === inner) {
@@ -211,7 +241,7 @@ function sequence(lines, cur, indent) {
     @param {Line[]} lines @param {{ pos: number }} cur @param {number} indent
     @param {Line} [own] the key's own line, when it carried a block scalar */
 function assign(map, m, line, lines, cur, indent, own) {
-  const key = m[1] !== undefined ? m[1] : m[2] !== undefined ? m[2].replace(/''/g, "'") : m[3].trim();
+  const key = m[1] !== undefined ? unescapeDouble(m[1]) : m[2] !== undefined ? m[2].replace(/''/g, "'") : m[3].trim();
   if (own && own.block !== undefined) {
     map[key] = own.block;
     return;
@@ -253,6 +283,7 @@ function mapping(lines, cur, indent) {
          Only valid as the whole value of the key above, which assign handles. */
       break;
     }
+    refuseAnchor(l.text, l.line);
     const m = KEY_RE.exec(l.text);
     if (!m) throw new YamlLiteError('expected "key: value"', l.line);
     cur.pos++;