fc8de100a3437b9fc33bdeb0f459ca2a93e2f318 max Wed Sep 9 08:14:32 2026 -0700 hgSession: address the code review of the new Sessions page Rename and unshare now keep the public listing's thumbnail with the session it belongs to. The picture's file name is built from the encoded session name, so renaming a listed session left the listing pointing at nothing and the old file behind, and dropping a session from the listing to a plain shared link kept the picture. The classic page had the same problem in a subtler form: it removed the thumbnail after the row had already been renamed, so the old file survived. Saving under a name that is already in use asks before it replaces that session, using the failIfExists reply that the top-right Share a link menu already relies on. The description and "only I can load it" steps that follow a save now report a failure instead of reloading in silence, and what thumbnailAdd has to say when it cannot build a picture reaches the user instead of being freed unread. A session description no longer travels through a title attribute. The tooltip machinery in utils.js inserts its text with innerHTML and an attribute is decoded on the way, so a description containing angle brackets was interpreted as markup rather than shown as typed. It is attached, escaped, after each table draw, which also gives the rows DataTables renders later the same styled mouseovers as the rest of the page. Also: the AJAX endpoints say so when there is no session by that name, instead of reporting a no-op as a success; the new page always offers its way back to the classic page, since the cart variable that got the user there sticks; and four unused CSS rules, a dead element lookup and a dead local are gone. hgConfCatalog cited the wrong ticket for the two sessionNewPage flags. refs #38180, refs #38157 diff --git src/hg/js/hgSession.js src/hg/js/hgSession.js index 1cba1a3abdd..0ed45401fa3 100644 --- src/hg/js/hgSession.js +++ src/hg/js/hgSession.js @@ -1,48 +1,50 @@ // hgSession.js - the experimental client-rendered "My Sessions" page. // // An opt-in modern alternative to the classic server-rendered hgSession page, applying hgBlat's // facelift strategy (#37996): hgSession.c emits the session list and page config as an inline JSON // global (hgSessionData) into an empty #sessionApp container, and this file builds the UI - a // save-current-view card, a searchable/sortable DataTable of saved sessions with inline // Overwrite/Share/Edit/Delete, and an "Advanced" panel for loading and backup. // // The inline table actions POST to small JSON endpoints in hgSession.c (hgS_doDeleteJson, etc.) and // update the table in place. Navigation actions (load a session, load from URL/file, save to file, // reset) are ordinary form submits/links against the existing hgSession actions. // // Styling: shared house-style components in gbModern.css (.gbPill, .gbCard, .gbModal*, .gbTable, // .gbBanner, .gbSection), session-specific layout in hgSession.css. -/* global $, hgSessionData, convertTitleTagsToMouseovers, htmlEncode, commify, gbShowTimingDialog */ +/* global $, hgSessionData, convertTitleTagsToMouseovers, titleTagToMouseover, addMouseover */ +/* global htmlEncode, commify, gbShowTimingDialog */ // Cart action variables (must match the hgs* defines in hgSession.h; hgSessionPrefix is "hgS_"). var SESS_ACT = { save: 'hgS_doSaveSessionJson', rename: 'hgS_doRenameSessionJson', del: 'hgS_doDeleteJson', share: 'hgS_doShareJson', gallery: 'hgS_doGalleryJson', overwrite: 'hgS_doOverwriteJson', describe: 'hgS_doDescribeJson' }; var SESS_P = { oldName: 'hgS_oldSessionName', newName: 'hgS_newSessionName', share: 'hgS_newSessionShare', descr: 'hgS_newSessionDescription', - shareAnon:'hgS_shareAnon' + shareAnon:'hgS_shareAnon', + failIfExists: 'hgS_failIfExists' }; var sessData = null; // set in sessionBuild: {config, sessions} var sessDt = null; // the DataTable API var sessSelectMode = false; // bulk-select (checkbox column) shown? function sessEnc(s) { // HTML-escape via the shared utils.js helper (escapes quotes too, so it is attribute-safe). return (typeof htmlEncode === 'function') ? htmlEncode(String(s == null ? '' : s)) : String(s); } function sessNum(n) { return (typeof commify === 'function') ? commify(n) : String(n); } @@ -141,49 +143,83 @@ // user can confirm with just the Enter key. function sessConfirm(opts) { var okClass = opts.okClass || 'primary'; sessModalOpen( '
' + sessEnc(opts.title) + '
' + '
' + opts.bodyHtml + '
' + '
' + '' + '
'); $('#sessCfCancel').on('click', sessModalClose); $('#sessCfOk').on('click', function() { opts.onOk(); }); document.getElementById('sessCfOk').focus(); } +// A notice with one button, for something the user has to see before the page reloads underneath +// them and takes the status line with it. bodyHtml is caller-built safe HTML. +function sessAlert(title, bodyHtml, onOk) { + sessModalOpen( + '
' + sessEnc(title) + '
' + + '
' + bodyHtml + '
' + + '
' + + '
'); + $('#sessAlertOk').on('click', function() { + sessModalClose(); + if (onOk) { onOk(); } + }); + document.getElementById('sessAlertOk').focus(); +} + // ---- session lookup / row helpers --------------------------------------- function sessByEnc(enc) { var list = sessData.sessions; for (var i = 0; i < list.length; i++) { if (list[i].encName === enc) { return list[i]; } } return null; } function sessRowByEnc(enc) { // Return the DataTables row API for the session with this encName, or null. var found = null; sessDt.rows().every(function() { if (this.data().encName === enc) { found = this; } }); return found; } +function sessApplyTooltips() { + // Runs after every table draw, because DataTables renders rows on demand (page two, a re-sort) + // and the one-time conversion utils.js does at page load never sees those. Two jobs: + // - give the plain title attributes in the new rows the same styled mouseovers as the rest of + // the page, skipping whatever has already been converted; + // - hand each info bubble its session description. The description is the user's own text and + // the tooltip is inserted with innerHTML, so it is passed already escaped: markup in a + // description then reads as the characters that were typed instead of being parsed as HTML. + if (typeof titleTagToMouseover !== 'function' || typeof addMouseover !== 'function') { return; } + var $table = $('#sessionAppTable'); + $table.find('[title]').each(function() { + if (this.title && this.getAttribute('mouseoverText') === null) { titleTagToMouseover(this); } + }); + $table.find('span.sessInfo[data-enc]').each(function() { + var row = sessByEnc(this.getAttribute('data-enc')); + if (row && row.description) { addMouseover(this, sessEnc(row.description)); } + }); +} + // ---- table cell rendering ------------------------------------------------ // Inline icons (Font Awesome solid paths, embedded as SVG so they do not depend on the site's Font // Awesome version). fill:currentColor picks up the button's text/danger color. var SESS_TRASH_SVG = ''; // Font Awesome "floppy-disk" (save) regular/outline path - reads more clearly as a save icon. var SESS_SAVE_SVG = ''; + // No title attribute here: the description is the user's own text and the tooltip machinery + // in utils.js inserts its text with innerHTML, so a title would have markup in a description + // parsed as HTML. sessApplyTooltips() attaches it, escaped, after the row is drawn. + html += ' '; } // Sessions are shared by default; mark only the exceptions: a lock for private, a badge for the // public gallery. A plain shared-by-link session gets no marker. if (row.shared === 0) { html += ' ' + SESS_LOCK_SVG + ''; } else if (row.shared >= 2) { html += ' Public'; } return html; } // ---- Overwrite ----------------------------------------------------------- function sessDoOverwrite(row) { @@ -319,31 +358,37 @@ function finish() { if (nameChanged) { // encName changes on rename; reload for authoritative state. window.location.reload(); } else { var r = sessRowByEnc(row.encName); if (r) { r.data(row).draw(false); } sessModalClose(); sessMsg('Saved changes to “' + row.name + '”.', 'ok'); } } function doRename() { if (nameChanged) { var rp = sessActParams(SESS_ACT.rename, row); rp[SESS_P.newName] = newName; - sessAjax(rp, finish, function(m) { sessEditError(m); }); + // A rename moves the public-listing thumbnail, which can fail on its own (a mirror with + // no ImageMagick); say so before the reload takes the message away. + sessAjax(rp, function(resp) { + if (resp && resp.warning) { + sessAlert('Session renamed', sessEnc(resp.warning), finish); + } else { finish(); } + }, function(m) { sessEditError(m); }); } else { finish(); } } function doPriv() { if (privChanged) { var sp = sessActParams(SESS_ACT.share, row); sp[SESS_P.share] = wantPrivate ? 0 : 1; sessAjax(sp, function(resp) { row.shared = resp.shared; doRename(); }, function(m) { sessEditError(m); }); } else { doRename(); } } function doDesc() { if (descChanged) { var dp = sessActParams(SESS_ACT.describe, row); dp[SESS_P.descr] = newDesc; sessAjax(dp, function() { row.description = newDesc; doPriv(); }, @@ -391,42 +436,45 @@ inp.focus(); inp.select(); if (navigator.clipboard) { navigator.clipboard.writeText(row.shareUrl); } else { try { document.execCommand('copy'); } catch (e) { /* ignore */ } } this.textContent = 'Copied'; }); $('#sessGalleryChk').on('change', function() { sessSetGallery(row, this.checked ? 1 : 0); }); } function sessShareErr(msg) { var err = document.getElementById('sessShareErr'); if (err) { err.style.display = 'block'; err.innerHTML = sessEnc(msg); } } function sessAfterSharedChange(row, newShared) { row.shared = newShared; - var c = document.getElementById('sessShareChk'); var g = document.getElementById('sessGalleryChk'); - if (c) { c.checked = (newShared >= 1); } if (g) { g.checked = (newShared >= 2); } var r = sessRowByEnc(row.encName); if (r) { r.data(row).draw(false); } } function sessSetGallery(row, want) { var p = sessActParams(SESS_ACT.gallery, row); p[SESS_P.share] = want; - sessAjax(p, function(resp) { sessAfterSharedChange(row, resp.shared); }, function(m) { + sessAjax(p, function(resp) { + sessAfterSharedChange(row, resp.shared); + // The listing itself worked; the picture for it may not have (e.g. a mirror without + // ImageMagick convert). Show what the server said rather than a bare success. + if (resp && resp.warning) { sessShareErr(resp.warning); } + }, function(m) { sessShareErr(m); document.getElementById('sessGalleryChk').checked = (row.shared >= 2); }); } // Build the base params for an action on a given session (decoded name; the CGI re-encodes it). function sessActParams(action, row) { var p = {}; p[action] = '1'; p[SESS_P.oldName] = row.name; return p; } // ---- Save current view --------------------------------------------------- @@ -438,61 +486,87 @@ var rand = sessRandomShareName(); sessConfirm({ title: 'Save without a name?', bodyHtml: 'You left the session name empty. Your session will be saved under the ' + 'randomly generated name ' + sessEnc(rand) + '.

You can also create ' + 'these quick share links any time from the Share a link option at the top ' + 'right of every Genome Browser page.', okLabel: 'Save session', onOk: function() { sessModalClose(); sessDoSaveWithName(rand); } }); return; } sessDoSaveWithName(name); } -function sessDoSaveWithName(name) { +function sessDoSaveWithName(name, allowOverwrite) { var priv = document.getElementById('sessSavePrivate').checked; var descEl = document.getElementById('sessSaveDesc'); var desc = descEl ? descEl.value.trim() : ''; var p = {}; p[SESS_ACT.save] = '1'; p[SESS_P.newName] = name; + // Saving under a name you are already using replaces that session's contents, so ask first. + // With failIfExists set the CGI answers {exists: true} instead of saving, which is how the + // top-right "Share a link" menu handles the same collision. + if (!allowOverwrite) { p[SESS_P.failIfExists] = '1'; } + // doSaveSessionJson always saves shared-by-link (the default); chain the optional description // and, if the user asked for "only I can load it", make it private, then reload to show the row. + // The session is saved by the time those run, so a failure in one of them has to be reported: a + // silent reload would leave a session sitting there shared by link, or with no description, and + // tell the user nothing. + function reload() { window.location.reload(); } + function partlySaved(problem) { + sessAlert('Session saved, with a problem', + 'Your session ' + sessEnc(name) + ' was saved, but ' + sessEnc(problem), + reload); + } function afterDesc() { - if (priv) { + if (!priv) { reload(); return; } var sp = {}; sp[SESS_ACT.share] = '1'; sp[SESS_P.oldName] = name; sp[SESS_P.share] = 0; - sessAjax(sp, function() { window.location.reload(); }, - function() { window.location.reload(); }); - } else { - window.location.reload(); - } + sessAjax(sp, reload, function(m) { + partlySaved('it could not be made private, so anyone with the link can still load it. ' + + m); + }); } - sessAjax(p, function() { - if (desc) { + function afterSave() { + if (!desc) { afterDesc(); return; } var dp = {}; dp[SESS_ACT.describe] = '1'; dp[SESS_P.oldName] = name; dp[SESS_P.descr] = desc; - sessAjax(dp, afterDesc, afterDesc); - } else { - afterDesc(); + sessAjax(dp, afterDesc, function(m) { + partlySaved('the description could not be saved. ' + m); + }); + } + sessAjax(p, function(resp) { + if (resp && resp.exists) { + sessConfirm({ + title: 'Replace this session?', + bodyHtml: 'You already have a session named ' + sessEnc(name) + '. Replacing ' + + 'it points that name at the view you are looking at now, and what the session ' + + 'held before is gone.', + okLabel: 'Replace it', + onOk: function() { sessModalClose(); sessDoSaveWithName(name, true); } + }); + return; } + afterSave(); }); } // ---- Advanced panel (navigation forms) ---------------------------------- function sessAdvancedHtml(C) { var sid = ''; var loadUser = ''; if (C.loggedIn) { loadUser = '
Load another user’s session' + '
' + sid + '' + '' + '