72de5a10dd13d893dd4111ac8acfdc29579760e8
jcasper
  Sat Apr 25 10:18:50 2026 -0700
Fixing a conflict in faceted composites between the use of primaryKey values as
data elements (for subtrack names) and the features around linking out (id|label stuff).  refs #36320

diff --git src/hg/js/facetedComposite.js src/hg/js/facetedComposite.js
index 0a54703ad1d..3b3c88af412 100644
--- src/hg/js/facetedComposite.js
+++ src/hg/js/facetedComposite.js
@@ -28,30 +28,39 @@
             })
             : fetch(fetchUrl, {
                 method: "GET",
                 headers: { "Content-Type": "application/x-www-form-urlencoded" },
             });
         return req.then(r => r.ok ? r.json() : null).catch(() => null);
     };
 
     const toTitleCase = str =>
           str.toLowerCase()
           .split(/[_\s-]+/)  // Split on underscore, space, or hyphen
           .map(word => word.charAt(0).toUpperCase() + word.slice(1)).join(" ");
 
     const escapeRegex = str => str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
 
+    // For primaryKey values that use the 'id|label' form, return just the id.
+    // The label is for display only; the cart and rowToIdx need the bare id.
+    const primaryKeyId = v => {
+        if (v == null) return v;
+        const s = String(v);
+        const bar = s.indexOf("|");
+        return bar >= 0 ? s.slice(0, bar) : s;
+    };
+
     const embeddedData = (() => {
         // get data that was embedded in the HTML here to use them as globals
         const dataTag = document.getElementById("app-data");
         return dataTag ? JSON.parse(dataTag.innerText) : "";
     })();
 
     // Store initial checkbox states for delta computation on server
     const initialState = {
         dataElements: new Set(),
         dataTypes: new Set()
     };
 
     function generateHTML() {
         const container = document.createElement("div");
         container.id = "myTag";
@@ -150,30 +159,35 @@
                 // urlTemplate is html-encoded server-side (htmlEncode in
                 // hgTrackUi.c), so it's safe to interpolate into an href.
                 col.render = (data, type) => {
                     if (type !== "display") return data;
                     if (data == null || data === "") return "";
                     const parts = String(data).split(",")
                                               .map(s => s.trim())
                                               .filter(Boolean);
                     return parts.map(tok => {
                         let idForUrl = tok, label = tok, encode = true;
                         const bar = tok.indexOf("|");
                         if (bar >= 0) {
                             idForUrl = tok.slice(0, bar);
                             label    = tok.slice(bar + 1);
                             encode   = false;
+                            // Strip enclosing quotes from the metadata.tsv
+                            if (label.length >= 2 &&
+                                label.startsWith('"') && label.endsWith('"')) {
+                                label = label.slice(1, -1);
+                            }
                         }
                         if (/^https?:/i.test(label)) encode = false;
                         const sub = encode ? encodeURIComponent(idForUrl) : idForUrl;
                         const href = urlTemplate.replace(/\$\$/g, sub);
                         return `<a href="${href}" target="_blank">${label}</a>`;
                     }).join(", ");
                 };
             }
             return col;
         });
 
         const checkboxColumn = {
             data: null,
             orderable: false,
             defaultContent: "",
@@ -660,31 +674,31 @@
                 // Get current data type selections
                 document.querySelectorAll("input.cbgroup").forEach(cb => {
                     if (cb.checked) {
                         currentDataTypes.push(cb.value);
                     }
                 });
                 // Require at least one data type when the selector exists
                 if (currentDataTypes.length === 0) {
                     alert("Please select at least one data type.");
                     return;  // abort submission
                 }
             }
 
             // Get current data element selections
             const currentDataElements = table.rows({selected: true}).data().toArray()
-                .map(obj => obj[primaryKey]);
+                .map(obj => primaryKeyId(obj[primaryKey]));
 
             // Enforce an upper bound on the number of tracks on at the same time.
             // This is imperfect when data types are present - some combinations might
             // have been manually hidden by the user.  But it should be a good ballpark.
             const trackLimit = 1000;
             if (hasDataTypes) {
                 if (currentDataTypes.length * currentDataElements.length > trackLimit) {
                     alert("You have turned on too many subtracks (over 1000) - please uncheck some.");
                     return;  // abort submission
                 }
             } else {
                 if (currentDataElements.length > trackLimit) {
                     alert("You have turned on too many subtracks (over 1000) - please uncheck some.");
                     return;  // abort submission
                 }
@@ -768,32 +782,41 @@
             })
             .then(tsvText => {  // metadata table is a TSV file to parse
                 loadOptional(colorSettingsUrl, hgsid, track).then(colorMap => {
                     const rows = tsvText.trim().split("\n");
                     const colNames = rows[0].split("\t");
                     if (!primaryKey)
                         throw new Error("trackDb setting 'primaryKey' is missing");
                     if (!colNames.includes(primaryKey))
                         throw new Error(`primaryKey '${primaryKey}' not found in metadata columns`);
                     const metadata = rows.slice(1).map(row => {
                         const values = row.split("\t");
                         const obj = {};
                         colNames.forEach((attrib, i) => { obj[attrib] = values[i]; });
                         return obj;
                     });
+                    // Commas in the primaryKey column are ambiguous: a row maps
+                    // to a single subtrack, and trackDb subtrack names can't
+                    // contain commas anyway. Fail loudly so the author notices.
+                    const badPk = metadata.find(row =>
+                        row[primaryKey] != null && String(row[primaryKey]).includes(","));
+                    if (badPk)
+                        throw new Error(
+                            `primaryKey column '${primaryKey}' contains a comma in value ` +
+                            `'${badPk[primaryKey]}'; commas are not allowed in primaryKey values`);
                     const rowToIdx = Object.fromEntries(
-                        metadata.map((row, i) => [row[primaryKey], i])
+                        metadata.map((row, i) => [primaryKeyId(row[primaryKey]), i])
                     );
                     colorMap = isValidColorMap(colorMap) ? colorMap : null;
                     const freshData = { metadata, rowToIdx, colNames, colorMap };
 
                     initAll(freshData);
                 });
             })
             .catch(err => {
                 const table = document.getElementById("theMetaDataTable");
                 if (table) {
                     table.innerHTML =
                         `<tr><td style="padding:20px;color:#a00;">` +
                         `Error loading metadata: ${err.message}</td></tr>`;
                 }
             });