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( '<div class="gbModalTitle">' + sessEnc(opts.title) + '</div>' + '<div class="gbModalText">' + opts.bodyHtml + '</div>' + '<div class="gbModalBtns">' + '<button type="button" class="gbPill" id="sessCfCancel">Cancel</button>' + '<button type="button" class="gbPill ' + okClass + '" id="sessCfOk">' + sessEnc(opts.okLabel || 'OK') + '</button></div>'); $('#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( + '<div class="gbModalTitle">' + sessEnc(title) + '</div>' + + '<div class="gbModalText">' + bodyHtml + '</div>' + + '<div class="gbModalBtns">' + + '<button type="button" class="gbPill primary" id="sessAlertOk">OK</button></div>'); + $('#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 = '<svg class="sessIcon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"' + ' aria-hidden="true"><path fill="currentColor" d="M135.2 17.7L128 32H32C14.3 32 0 46.3 0 64S14.3' + ' 96 32 96H416c17.7 0 32-14.3 32-32s-14.3-32-32-32H320l-7.2-14.3C307.4 6.8 296.3 0 284.2 0H163.8' + 'c-12.1 0-23.2 6.8-28.6 17.7zM416 128H32L53.2 467c1.6 25.3 22.6 45 47.9 45H346.9c25.3 0 46.3-19.7' + ' 47.9-45L416 128z"/></svg>'; // Font Awesome "floppy-disk" (save) regular/outline path - reads more clearly as a save icon. var SESS_SAVE_SVG = '<svg class="sessIcon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 640 640"' + ' aria-hidden="true"><path fill="currentColor" d="M160 144C151.2 144 144 151.2 144 160L144 480C144' + ' 488.8 151.2 496 160 496L480 496C488.8 496 496 488.8 496 480L496 237.3C496 233.1 494.3 229 491.3' + ' 226L416 150.6L416 240C416 257.7 401.7 272 384 272L224 272C206.3 272 192 257.7 192 240L192 144L160' + ' 144zM240 144L240 224L368 224L368 144L240 144zM96 160C96 124.7 124.7 96 160 96L402.7 96C419.7 96' + @@ -202,31 +238,34 @@ return '<button type="button" class="gbPill" data-act="share" data-enc="' + e + '" ' + 'title="Copy a shareable link or change who can see this session">Share</button>' + '<button type="button" class="gbPill" data-act="edit" data-enc="' + e + '" ' + 'title="Rename this session or edit its description">Edit</button>' + '<button type="button" class="gbPill" data-act="overwrite" data-enc="' + e + '" ' + 'aria-label="Overwrite with current view" ' + 'title="Overwrite this session with your current browser view">' + SESS_SAVE_SVG + '</button>' + '<button type="button" class="gbPill danger" data-act="delete" data-enc="' + e + '" ' + 'aria-label="Delete this session" title="Delete this session">' + SESS_TRASH_SVG + '</button>'; } function sessNameCellHtml(row) { var html = '<a href="' + sessEnc(row.shareUrl) + '" ' + 'title="Load this session in the Genome Browser">' + sessEnc(row.name) + '</a>'; if (row.description) { - html += ' <span class="sessInfo" title="' + sessEnc(row.description) + '">ⓘ</span>'; + // 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 += ' <span class="sessInfo" data-enc="' + sessEnc(row.encName) + '">ⓘ</span>'; } // 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 += ' <span class="sessLockWrap" title="Private — only you can load this session">' + SESS_LOCK_SVG + '</span>'; } else if (row.shared >= 2) { html += ' <span class="sessPublic" title="Listed in the Public Sessions gallery">Public</span>'; } 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 <b>' + sessEnc(rand) + '</b>.<br><br>You can also create ' + 'these quick share links any time from the <b>Share a link</b> 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 <b>' + sessEnc(name) + '</b> 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 <b>' + sessEnc(name) + '</b>. 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 = '<input type="hidden" name="' + sessEnc(C.cartVar) + '" value="' + sessEnc(C.hgsid) + '">'; var loadUser = ''; if (C.loggedIn) { loadUser = '<div class="sessAdvItem"><span class="lab">Load another user’s session</span>' + '<form class="sessAdvRow" action="hgSession" method="POST">' + sid + '<input class="sessAdvInput" type="text" name="hgS_otherUserName" placeholder="User">' + '<input class="sessAdvInput" type="text" name="hgS_otherUserSessionName" placeholder="Session name">' + '<button type="submit" class="gbPill" name="hgS_doOtherUser" value="submit" ' + @@ -657,30 +731,31 @@ }); } // Advanced toggle. $('#sessAdvHead').on('click', function() { var body = document.getElementById('sessAdvBody'); var open = body.style.display !== 'none'; body.style.display = open ? 'none' : 'grid'; $(this).find('.caret').html(open ? '▸' : '▾'); }); // Session table. if (C.loggedIn) { sessBuildTable(); } if (typeof convertTitleTagsToMouseovers === 'function') { convertTitleTagsToMouseovers(); } + if (C.loggedIn) { sessApplyTooltips(); } // Timing report (only when loaded with &measureTiming=1): a pill that opens the shared dialog. if (sessData.timing) { var clientRows = [{ label: 'build page (JS)', ms: Math.round(performance.now() - tBuildStart) }]; var pill = document.createElement('button'); pill.type = 'button'; pill.className = 'gbPill'; pill.id = 'sessTimingBtn'; pill.innerHTML = '⏱ Timing'; pill.title = 'Show where this page spent its time (server and browser)'; pill.addEventListener('click', function() { gbShowTimingDialog(sessData.timing, clientRows); }); var bar = document.querySelector('#sessionApp .sessToolbar') || @@ -775,30 +850,33 @@ '<span title="' + sessEnc(row.createdFull || row.created) + '">' + sessEnc(row.created) + '</span>' : row.createdEpoch; } }, { title: 'Last used', data: 'lastUseEpoch', render: function(d, type, row) { return (type === 'display') ? '<span title="' + sessEnc(row.lastUse) + '">' + sessEnc(row.lastUseDate || row.lastUse) + '</span>' : row.lastUseEpoch; } }, { title: 'Views', data: 'useCount', render: function(d, type) { return (type === 'display') ? sessNum(d) : d; } }, { title: 'Actions', className: 'sessActionsCol', data: null, orderable: false, searchable: false, render: function(d, type, row) { return (type === 'display') ? sessActionsHtml(row) : ''; } } ] }); + // Every redraw brings rows the tooltip conversion has not seen yet. + $('#sessionAppTable').on('draw.dt', sessApplyTooltips); + // Delegate the row action buttons. $('#sessionAppTable tbody').on('click', 'button[data-act]', function() { var act = this.getAttribute('data-act'); var row = sessByEnc(this.getAttribute('data-enc')); if (!row) { return; } if (act === 'overwrite') { sessOpenOverwrite(row); } else if (act === 'share') { sessOpenShare(row); } else if (act === 'edit') { sessOpenEdit(row); } else if (act === 'delete') { sessOpenDelete(row); } }); // Bulk-select control: a "Select" button after the length dropdown reveals a checkbox column and // becomes a primary "Delete all selected" button. var selBtn = document.createElement('button'); selBtn.type = 'button';