Skip to content

Commit 1d68ed4

Browse files
Kunal241207Kunal241207
authored
Harden JS editor console rendering and modernize Lilypond clipboard copy (#5731)
* Render console output via DOM nodes and use Clipboard API with fallback * Harden console output and clipboard copy * Apply Prettier formatting
1 parent d957f14 commit 1d68ed4

File tree

4 files changed

+154
-16
lines changed

4 files changed

+154
-16
lines changed

‎js/SaveInterface.js‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -784,20 +784,49 @@ class SaveInterface {
784784

785785
afterSaveLilypondLY(lydata, filename) {
786786
filename = docById("fileName").value;
787-
if (platform.FF) {
788-
// eslint-disable-next-line no-console
789-
console.debug('execCommand("copy") does not work on FireFox');
790-
} else {
787+
const showCopiedMessage = () => {
788+
this.activity.textMsg(
789+
_("The Lilypond code is copied to clipboard. You can paste it here: ") +
790+
"<a href='http://hacklily.org' target='_blank'>http://hacklily.org</a> "
791+
);
792+
};
793+
794+
const legacyCopy = () => {
791795
const tmp = jQuery("<textarea />").appendTo(document.body);
792796
tmp.val(lydata);
793797
tmp.select();
794798
tmp[0].setSelectionRange(0, lydata.length);
795-
document.execCommand("copy");
799+
let copied = false;
800+
try {
801+
copied = document.execCommand("copy");
802+
} catch (e) {
803+
copied = false;
804+
}
796805
tmp.remove();
797-
this.activity.textMsg(
798-
_("The Lilypond code is copied to clipboard. You can paste it here: ") +
799-
"<a href='http://hacklily.org' target='_blank'>http://hacklily.org</a> "
800-
);
806+
return copied;
807+
};
808+
809+
if (navigator.clipboard && window.isSecureContext) {
810+
navigator.clipboard
811+
.writeText(lydata)
812+
.then(showCopiedMessage)
813+
.catch(err => {
814+
const copied = legacyCopy();
815+
if (copied) {
816+
showCopiedMessage();
817+
} else {
818+
// eslint-disable-next-line no-console
819+
console.debug("Clipboard copy failed:", err);
820+
}
821+
});
822+
} else {
823+
const copied = legacyCopy();
824+
if (copied) {
825+
showCopiedMessage();
826+
} else {
827+
// eslint-disable-next-line no-console
828+
console.debug("Clipboard copy failed");
829+
}
801830
}
802831
this.download("ly", "data:text;utf8," + encodeURIComponent(lydata), filename);
803832
}

‎js/__tests__/SaveInterface.test.js‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ describe("afterSaveMIDI", () => {
273273
global.activity = {
274274
logo: {
275275
_midiData: {
276-
"0": [
276+
0: [
277277
{
278278
note: ["G4"],
279279
duration: 4,
@@ -514,8 +514,13 @@ describe("saveWAV & saveABC methods", () => {
514514

515515
describe("saveLilypond Methods", () => {
516516
let activity, saveInterface, mockActivity, mockDocById;
517+
let originalClipboard;
518+
let originalSecureContext;
517519

518520
beforeEach(() => {
521+
jest.useRealTimers();
522+
originalClipboard = navigator.clipboard;
523+
originalSecureContext = window.isSecureContext;
519524
// Set up the DOM structure
520525
document.body.innerHTML = `
521526
<div id="lilypondModal" style="display: none;">
@@ -553,6 +558,7 @@ describe("saveLilypond Methods", () => {
553558
return { on: jest.fn() };
554559
}
555560
return {
561+
0: { setSelectionRange: jest.fn() },
556562
appendTo: jest.fn().mockReturnThis(),
557563
val: jest.fn().mockReturnThis(),
558564
select: jest.fn(),
@@ -661,6 +667,15 @@ describe("saveLilypond Methods", () => {
661667

662668
afterEach(() => {
663669
jest.clearAllMocks();
670+
if (originalClipboard === undefined) {
671+
delete navigator.clipboard;
672+
} else {
673+
navigator.clipboard = originalClipboard;
674+
}
675+
Object.defineProperty(window, "isSecureContext", {
676+
value: originalSecureContext,
677+
configurable: true
678+
});
664679
});
665680

666681
it("should save a Lilypond file with default settings", () => {
@@ -749,6 +764,58 @@ describe("saveLilypond Methods", () => {
749764
// Verify activity.save.download is not called
750765
expect(activity.save.download).not.toHaveBeenCalled();
751766
});
767+
768+
it("should show copied message when Clipboard API succeeds", async () => {
769+
const writeText = jest.fn().mockResolvedValue();
770+
navigator.clipboard = { writeText };
771+
Object.defineProperty(window, "isSecureContext", { value: true, configurable: true });
772+
773+
saveInterface.afterSaveLilypondLY(lydata, filename);
774+
await writeText.mock.results[0].value;
775+
776+
expect(writeText).toHaveBeenCalledWith(lydata);
777+
expect(document.execCommand).not.toHaveBeenCalled();
778+
expect(mockActivity.textMsg).toHaveBeenCalled();
779+
});
780+
781+
it("should fall back to legacy copy when Clipboard API fails", async () => {
782+
const writeText = jest.fn().mockRejectedValue(new Error("denied"));
783+
navigator.clipboard = { writeText };
784+
Object.defineProperty(window, "isSecureContext", { value: true, configurable: true });
785+
document.execCommand.mockReturnValue(true);
786+
787+
saveInterface.afterSaveLilypondLY(lydata, filename);
788+
await writeText.mock.results[0].value.catch(() => {});
789+
790+
expect(writeText).toHaveBeenCalledWith(lydata);
791+
expect(document.execCommand).toHaveBeenCalledWith("copy");
792+
expect(mockActivity.textMsg).toHaveBeenCalled();
793+
});
794+
795+
it("should not show copied message when all copy methods fail", async () => {
796+
const writeText = jest.fn().mockRejectedValue(new Error("denied"));
797+
navigator.clipboard = { writeText };
798+
Object.defineProperty(window, "isSecureContext", { value: true, configurable: true });
799+
document.execCommand.mockReturnValue(false);
800+
801+
saveInterface.afterSaveLilypondLY(lydata, filename);
802+
await writeText.mock.results[0].value.catch(() => {});
803+
804+
expect(writeText).toHaveBeenCalledWith(lydata);
805+
expect(document.execCommand).toHaveBeenCalledWith("copy");
806+
expect(mockActivity.textMsg).not.toHaveBeenCalled();
807+
});
808+
809+
it("should use legacy copy when Clipboard API is unavailable", () => {
810+
delete navigator.clipboard;
811+
Object.defineProperty(window, "isSecureContext", { value: true, configurable: true });
812+
document.execCommand.mockReturnValue(true);
813+
814+
saveInterface.afterSaveLilypondLY(lydata, filename);
815+
816+
expect(document.execCommand).toHaveBeenCalledWith("copy");
817+
expect(mockActivity.textMsg).toHaveBeenCalled();
818+
});
752819
});
753820

754821
describe("MXML Methods", () => {

‎js/__tests__/jseditor_console.test.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
const { JSEditor } = require("../widgets/jseditor");
2+
3+
describe("JSEditor console rendering", () => {
4+
beforeEach(() => {
5+
document.body.innerHTML = '<div id="editorConsole"></div>';
6+
global.docById = jest.fn(id => document.getElementById(id));
7+
});
8+
9+
afterEach(() => {
10+
jest.clearAllMocks();
11+
});
12+
13+
it("renders messages as plain text with line breaks", () => {
14+
JSEditor.logConsole("<b>hello</b>", "red");
15+
JSEditor.logConsole("second", "blue");
16+
17+
const consoleEl = document.getElementById("editorConsole");
18+
19+
expect(consoleEl.textContent).toBe("<b>hello</b>second");
20+
expect(consoleEl.querySelector("b")).toBeNull();
21+
expect(consoleEl.querySelectorAll("br").length).toBe(1);
22+
});
23+
24+
it("clears console content", () => {
25+
JSEditor.logConsole("message", "red");
26+
JSEditor.clearConsole();
27+
28+
const consoleEl = document.getElementById("editorConsole");
29+
expect(consoleEl.textContent).toBe("");
30+
expect(consoleEl.childNodes.length).toBe(0);
31+
});
32+
});

‎js/widgets/jseditor.js‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -836,10 +836,15 @@ class JSEditor {
836836
*/
837837
static logConsole(message, color) {
838838
if (color === undefined) color = "midnightblue";
839-
if (docById("editorConsole")) {
840-
if (docById("editorConsole").innerHTML !== "")
841-
docById("editorConsole").innerHTML += "</br>";
842-
docById("editorConsole").innerHTML += `<span style="color: ${color}">${message}</span>`;
839+
const consoleEl = docById("editorConsole");
840+
if (consoleEl) {
841+
if (consoleEl.childNodes.length > 0) {
842+
consoleEl.appendChild(document.createElement("br"));
843+
}
844+
const line = document.createElement("span");
845+
line.style.color = color;
846+
line.textContent = message;
847+
consoleEl.appendChild(line);
843848
} else {
844849
// console.error("EDITOR MISSING!");
845850
}
@@ -848,8 +853,9 @@ class JSEditor {
848853
}
849854

850855
static clearConsole() {
851-
if (docById("editorConsole")) {
852-
docById("editorConsole").innerHTML = "";
856+
const consoleEl = docById("editorConsole");
857+
if (consoleEl) {
858+
consoleEl.textContent = "";
853859
}
854860
}
855861

@@ -1162,3 +1168,7 @@ class JSEditor {
11621168
JSEditor.logConsole("Status window opened.", "green");
11631169
}
11641170
}
1171+
1172+
if (typeof module !== "undefined" && module.exports) {
1173+
module.exports = { JSEditor };
1174+
}

0 commit comments

Comments
 (0)