d033cea2063e9362949baf5b4d8b837597173a0d
max
Thu Sep 10 05:16:05 2026 -0700
Address the code review of the Sep 9 commits
Faceted composite: text that comes from a hub - a metadata column's
description, the name and title of a data type, and the values quoted back in
the "could not load the metadata" row - is put on the page as text rather than
as markup. The three places built their markup from template strings, so a
value carrying angle brackets or a quote was read as HTML: the column
description now goes through the shared htmlEncode() once where the header is
parsed, and the other two build their elements as nodes. The error row reads
better for it as well, since a value with brackets in it used to disappear from
the message that was meant to show it.
The saved UI state keys on the assembly as well as the metadata id. localStorage
is per-origin, so two assemblies whose tracks share a name were sharing one
entry, and a row order dragged on one came back on the other over a different
set of samples. hgTrackUi passes the database down for it. State saved under the
old key is dropped, which costs a facet selection or a page length.
Imprinting: the five subtrack description pages link back to the container as
hgTrackUi?db=$db&g=$parentTrack, without the hgsid. Native trackDb html is
substituted by hgTrackDb as it loads the table, where there is no cart, so
${hgsid} came out empty and the link read 'hgsid=&g=...'. Matches what the
Fiber-seq pages already do. The makeDoc note that described the old form is
updated with the reason.
UniProt otto: README.txt lists all eight things that reach runLog.txt. It had
four, and was missing LOCKED, along with PREFLIGHT-FAIL, END and INTERRUPTED.
refs #36210
refs #37599
refs #38300
diff --git src/hg/js/facetedComposite.js src/hg/js/facetedComposite.js
index 6fa665b7b8b..3ccac1a4448 100644
--- src/hg/js/facetedComposite.js
+++ src/hg/js/facetedComposite.js
@@ -184,31 +184,35 @@
// Set by initTable() once the "Group by" tabs exist, read by initSubmit().
// Returns null for a composite without data types, where there is nothing
// to group and no control is drawn.
let getGroupBy = () => null;
// How this picker was last left: which facet boxes were ticked, what was
// typed in each column's search box, which tab was showing, how many rows
// per page, and a hand-dragged row order if there is one. None of it
// changes what the Genome Browser draws, so it stays out of the cart:
// putting it there would grow every session, and a manual order over a
// table the size of Methbase's 6500 rows would be a large value to carry
// around for a display preference. The sort column is the exception and
// does live in the cart, as facetSortOrder, because it also sets the track
// order in the image.
- const uiStateKey = `facetedComposite.${embeddedData.mdid}`;
+ // The key carries the assembly as well as the metadata id: localStorage is
+ // per-origin, so two assemblies whose hubs happen to use the same track
+ // name would otherwise share one saved state, and a row order dragged for
+ // one would come back on the other over a different set of samples.
+ const uiStateKey = `facetedComposite.${embeddedData.db || ""}.${embeddedData.mdid}`;
function loadUiState() {
// A private window throws on access rather than returning null, and a
// half-written value from an older build should not break the page.
try {
const raw = localStorage.getItem(uiStateKey);
const state = raw ? JSON.parse(raw) : null;
return (state && typeof state === "object") ? state : {};
} catch (e) {
return {};
}
}
// A page length the user picked wins. Otherwise paginating a table that
// would nearly fit anyway just hides rows behind a menu, so show everything
@@ -291,35 +295,47 @@
// Skip if no dataTypes defined or empty object
if (!embeddedData.dataTypes || Object.keys(embeddedData.dataTypes).length === 0) {
return;
}
const selector = document.getElementById("dataTypeSelector");
selector.appendChild(Object.assign(document.createElement("label"), {
innerHTML: "<b>Data types shown in the browser:</b>",
}));
selector.appendChild(createInfoIcon(
"Each sample has several data type tracks in the Genome Browser. " +
"Check the boxes of the types of tracks you wish to show when a " +
"sample row is selected below."));
+ // Built as nodes rather than from a template string. Both the name and
+ // the title come from the trackDb 'dataTypes' setting, which on a hub is
+ // whatever the hub author wrote, so they go in as a property value and a
+ // text node instead of being interpolated into HTML.
+ // The leading space is what the old template literal's newline and
+ // indentation collapsed to, and is what separates one checkbox from the
+ // one before it.
Object.keys(embeddedData.dataTypes).forEach(name => {
const label = document.createElement("label");
const dataType = embeddedData.dataTypes[name];
- label.innerHTML = `
- <input type="checkbox" class="cbgroup" value="${name}">${dataType.title}`;
+ const cb = document.createElement("input");
+ cb.type = "checkbox";
+ cb.className = "cbgroup";
+ cb.value = name;
+ label.appendChild(document.createTextNode(" "));
+ label.appendChild(cb);
+ label.appendChild(document.createTextNode(dataType.title));
selector.appendChild(label);
});
const selectedDataTypes = new Set( // get dataTypes selected initially
Object.entries(embeddedData.dataTypes).filter(([_, val]) => val.active === 1)
.map(([key]) => key)
);
// initialize data type checkboxes (using class instead of 'name')
document.querySelectorAll("input.cbgroup")
.forEach(cb => { cb.checked = selectedDataTypes.has(cb.value); });
// Turning a data type on is a request to see it, so take the container
// out of hide the same way selecting a sample does. These boxes were
// otherwise only read at submit time.
document.querySelectorAll("input.cbgroup").forEach(cb => {
cb.addEventListener("change", () => {
@@ -1397,31 +1413,35 @@
.then(tsvText => { // metadata table is a TSV file to parse
loadOptional(colorSettingsUrl, hgsid, track).then(colorMap => {
const rows = tsvText.trim().split("\n");
// A header cell may carry an optional longer description
// after a '|', e.g. "Sample_class|HPRC = ...". Only the
// name part is the column name, because it is also the key
// every row object is looked up by.
const rawColNames = parseTsvRow(rows[0]);
const colNames = [];
const colDescriptions = {};
rawColNames.forEach(raw => {
const bar = raw.indexOf("|");
const name = (bar < 0 ? raw : raw.slice(0, bar)).trim();
colNames.push(name);
if (bar >= 0) {
- const desc = raw.slice(bar + 1).trim();
+ // The description comes from the hub's metadata
+ // file and is shown in a tooltip, which renders as
+ // HTML, so it is encoded here, once, rather than at
+ // each of the two places that display it.
+ const desc = htmlEncode(raw.slice(bar + 1).trim());
if (desc) colDescriptions[name] = desc;
}
});
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 = parseTsvRow(row);
const obj = {};
colNames.forEach((attrib, i) => { obj[attrib] = values[i] ?? ""; });
return obj;
});
// Each primaryKey cell must map to exactly one subtrack.
const badPk = metadata.find(row =>
@@ -1432,31 +1452,44 @@
`'${badPk[primaryKey]}'; only one value is allowed per primaryKey cell`);
const rowToIdx = Object.fromEntries(
metadata.map((row, i) => [primaryKeyId(row[primaryKey]), i])
);
colorMap = isValidColorMap(colorMap) ? colorMap : null;
const freshData = { metadata, rowToIdx, colNames, colorMap,
colDescriptions };
initAll(freshData);
});
})
.catch(err => {
hideLoading(); // stop the spinner before showing the error
const table = document.getElementById("theMetaDataTable");
if (table) {
- table.innerHTML =
- `<tr><td style="padding:20px;color:#a00;">` +
- `Error loading metadata: ${err.message}</td></tr>`;
+ // The message names the trackDb primaryKey and can quote a
+ // metadata cell back, both of which come from the hub, so
+ // it goes in as a text node. That also keeps a value with
+ // angle brackets in it readable, where before the markup
+ // swallowed the very value the reader needs to see. The
+ // tbody is what the HTML parser used to add on its own.
+ const cell = document.createElement("td");
+ cell.style.padding = "20px";
+ cell.style.color = "#a00";
+ cell.appendChild(document.createTextNode(
+ `Error loading metadata: ${err.message}`));
+ const row = document.createElement("tr");
+ row.appendChild(cell);
+ const body = document.createElement("tbody");
+ body.appendChild(row);
+ table.replaceChildren(body);
}
});
} // end loadDataAndInit
document.addEventListener("keydown", e => { // block accidental submit
if (e.key === "Enter") { e.preventDefault(); e.stopPropagation(); }
}, true);
generateHTML();
showLoading(); // show spinner immediately, before the metadata fetch
loadDataAndInit();
});