Bug 40383: (follow-up) Address code review concerns

- Move TT interface/theme variables into a separate <script> tag to avoid
  mixing template variables with external JavaScript file scope
- Replace local escapeHtml() functions in edifact_interchange.js and
  edifact_errors.js with String.prototype.escapeHtml already defined
  in staff-global.js

Sponsored-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-03-09 07:46:32 +00:00
parent e9045ddedc
commit 631498e94e
No known key found for this signature in database
GPG Key ID: 2D86E0EAEC83711B
3 changed files with 20 additions and 39 deletions

View File

@ -47,19 +47,23 @@
[% INCLUDE 'datatables.inc' %]
[% Asset.js("js/modals/edifact_interchange.js") | $raw %]
[% Asset.js("js/modals/edifact_errors.js") | $raw %]
<script nonce="[% Koha.CSPNonce | $raw %]">
var interface = "[% interface | html %]";
var theme = "[% theme | html %]";
</script>
<script nonce="[% Koha.CSPNonce | $raw %]">
$(document).ready(function () {
// Initialize enhanced EDIFACT display
EdifactDisplay.init({
modalId: '#EDI_modal',
modalId: "#EDI_modal",
expandByDefault: true,
showElementDetails: false
showElementDetails: false,
});
// Get CSRF token once for reuse
var csrfToken = $('meta[name="csrf-token"]').attr("content") ? $('meta[name="csrf-token"]').attr("content").trim() : '';
var csrfToken = $('meta[name="csrf-token"]').attr("content") ? $('meta[name="csrf-token"]').attr("content").trim() : "";
var edi_msgs_table_url = '/api/v1/acquisitions/edifiles?';
var edi_msgs_table_url = "/api/v1/acquisitions/edifiles?";
var edi_msgs_table = $("#edi_msgs").kohaTable(
{
ajax: {
@ -94,7 +98,7 @@
// JSON encode the errors instead of constructing HTML
let errorsJson = JSON.stringify(row.errors);
// Escape quotes for HTML attribute
errorsJson = errorsJson.replace(/"/g, '&quot;');
errorsJson = errorsJson.replace(/"/g, "&quot;");
rendered = row.status + ' (<a href="#" data-bs-toggle="modal" data-bs-target="#EDI_errors_modal" data-bs-errors="' + errorsJson + '" data-bs-filename="' + row.filename + '">' + _("with errors") + "</a>)";
} else {
rendered = row.status;
@ -158,10 +162,11 @@
// Import button
if (row.status == "new") {
result += '<a class="btn btn-default btn-xs import_msg" href="/cgi-bin/koha/acqui/edifactmsgs.pl?op=import&amp;message_id=' + encodeURIComponent(row.id) + '"><i class="fa fa-cog"></i> ' + _("Import") + "</a>";
result +=
'<a class="btn btn-default btn-xs import_msg" href="/cgi-bin/koha/acqui/edifactmsgs.pl?op=import&amp;message_id=' + encodeURIComponent(row.id) + '"><i class="fa fa-cog"></i> ' + _("Import") + "</a>";
}
result += '</div>';
result += "</div>";
return result;
},
},

View File

@ -57,7 +57,7 @@ const EdifactErrorsDisplay = (() => {
} catch (e) {
console.error("Error parsing errors JSON:", e);
modalBody.html(
`<div class="alert alert-danger">Error loading error details: ${escapeHtml(e.message)}</div>`
`<div class="alert alert-danger">Error loading error details: ${e.message.escapeHtml()}</div>`
);
}
} else {
@ -76,19 +76,19 @@ const EdifactErrorsDisplay = (() => {
.map(
error => `
<li class="list-group-item list-group-item-danger">
<strong>Error:</strong> ${escapeHtml(error.details || "Unknown error")}
<strong>Error:</strong> ${(error.details || "Unknown error").escapeHtml()}
${
error.section
? `
<br><small><strong>Section:</strong></small>
<pre class="mt-1 mb-0" style="font-size: 0.85em; background-color: #f8f9fa; padding: 0.5rem; border-radius: 0.25rem;">${escapeHtml(error.section)}</pre>
<pre class="mt-1 mb-0" style="font-size: 0.85em; background-color: #f8f9fa; padding: 0.5rem; border-radius: 0.25rem;">${error.section.escapeHtml()}</pre>
`
: ""
}
${
error.date
? `
<br><small class="text-muted"><strong>Date:</strong> ${escapeHtml(error.date)}</small>
<br><small class="text-muted"><strong>Date:</strong> ${error.date.escapeHtml()}</small>
`
: ""
}
@ -113,18 +113,6 @@ const EdifactErrorsDisplay = (() => {
filenameSpan.text("");
};
const escapeHtml = str => {
if (!str) return "";
const escapeMap = {
"&": "&amp;",
"<": "&lt;",
">": "&gt;",
'"': "&quot;",
"'": "&#039;",
};
return str.replace(/[&<>"']/g, match => escapeMap[match]);
};
return { init };
})();

View File

@ -205,7 +205,7 @@ const EdifactDisplay = (() => {
const tag = segment.tag || "";
const content = (segment.raw || "").substring(tag.length);
lines.push(
`<div class="segment-line${focusClass}"${messageAttr}><span class="segment-tag">${escapeHtml(tag)}</span>${escapeHtml(content)}</div>`
`<div class="segment-line${focusClass}"${messageAttr}><span class="segment-tag">${tag.escapeHtml()}</span>${content.escapeHtml()}</div>`
);
});
@ -935,14 +935,14 @@ const EdifactDisplay = (() => {
if (boldTag && content && content.length >= 3) {
const tag = content.substring(0, 3);
const rest = content.substring(3);
div.innerHTML = `<i class="fa fa-chevron-down me-1"></i> <span class="segment-tag">${escapeHtml(tag)}</span>${escapeHtml(rest)}`;
div.innerHTML = `<i class="fa fa-chevron-down me-1"></i> <span class="segment-tag">${tag.escapeHtml()}</span>${rest.escapeHtml()}`;
} else {
div.innerHTML = `<i class="fa fa-chevron-down me-1"></i> ${escapeHtml(content)}`;
div.innerHTML = `<i class="fa fa-chevron-down me-1"></i> ${content.escapeHtml()}`;
}
} else if (boldTag && content && content.length >= 3) {
const tag = content.substring(0, 3);
const rest = content.substring(3);
div.innerHTML = `<span class="segment-tag">${escapeHtml(tag)}</span>${escapeHtml(rest)}`;
div.innerHTML = `<span class="segment-tag">${tag.escapeHtml()}</span>${rest.escapeHtml()}`;
} else {
div.textContent = content;
}
@ -1004,18 +1004,6 @@ const EdifactDisplay = (() => {
});
};
const escapeHtml = str => {
if (!str) return "";
const escapeMap = {
"&": "&amp;",
"<": "&lt;",
">": "&gt;",
'"': "&quot;",
"'": "&#039;",
};
return str.replace(/[&<>"']/g, match => escapeMap[match]);
};
return {
init,
showMessage,