Skip to content

Commit c393edb

Browse files
committed
fix: replace SVG RECORDBUTTON with Material Icons ligature in toolbar
Record button used an SVG string set via innerHTML, which broke when PR 7757 converted innerHTML to textContent for security. Instead of reverting to innerHTML, move the icon to index.html as a static Material Icons ligature element, matching how Play and Stop work. - Add fiber_manual_record to #record in HTML - Remove dynamic icon creation from toolbar-ui.js - Refactor recorder.js to use CSS class on #record instead of targeting the now-removed rec_inside SVG element - Add #record.recording CSS for blink animation and red color - Remove dead RECORDBUTTON SVG constant from artwork.js - Remove unused RECORDBUTTON imports from toolbar-ui.js and turtledefs.js - Update tests to reflect static icon and class-based recording state
1 parent 3aa560a commit c393edb

9 files changed

Lines changed: 13 additions & 33 deletions

File tree

css/activities.css

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,8 @@ body:not(.dark):not(.highcontrast) #helpfulSearchDiv {
448448
line-height: 30px;
449449
}
450450

451-
.blink {
451+
#record.recording .material-icons {
452+
color: red;
452453
animation: blink-animation 1.5s infinite;
453454
}
454455

index.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,8 @@
549549
data-tooltip="Record"
550550
role="button"
551551
tabindex="0"
552-
></a>
552+
><i class="material-icons main">fiber_manual_record</i></a
553+
>
553554
<a
554555
id="recordDropdownArrow"
555556
class="left dropdown-trigger"

js/__tests__/toolbar.test.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -584,10 +584,7 @@ describe("Toolbar Class", () => {
584584
test("updateRecordButton hides record button", () => {
585585
const recordButton = {
586586
classList: { add: jest.fn() },
587-
style: { display: "" },
588-
innerHTML: "",
589-
textContent: "",
590-
appendChild: jest.fn()
587+
style: { display: "" }
591588
};
592589
global.docById.mockReturnValue(recordButton);
593590
global.fnBrowserDetect = jest.fn(() => "firefox");
@@ -597,14 +594,9 @@ describe("Toolbar Class", () => {
597594
});
598595

599596
test("updateRecordButton keeps only one outside-click listener and dispose removes it", () => {
600-
global.RECORDBUTTON = "fiber_manual_record";
601-
602597
const recordButton = {
603598
classList: { add: jest.fn(), remove: jest.fn() },
604599
style: { display: "" },
605-
innerHTML: "",
606-
textContent: "",
607-
appendChild: jest.fn(),
608600
onclick: null
609601
};
610602

js/__tests__/turtledefs.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ describe("Music Blocks mode (_THIS_IS_TURTLE_BLOCKS_ = false)", () => {
101101
"WRAPTURTLEBUTTON",
102102
"MOUSEPALETTEICON",
103103
"FULLSCREENBUTTON",
104-
"RECORDBUTTON",
105104
"PLUGINSBUTTON",
106105
"OPENMERGEBUTTON",
107106
"PITCHPREVIEWBUTTON",

js/activity/__tests__/recorder.test.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
describe("recorder", () => {
22
let mockStart;
3-
let mockRecInside;
43

54
beforeEach(() => {
65
jest.useFakeTimers();
@@ -15,17 +14,12 @@ describe("recorder", () => {
1514
addEventListener: jest.fn(),
1615
removeEventListener: jest.fn(),
1716
dispatchEvent: jest.fn(),
18-
_recordHandler: null
19-
};
20-
21-
mockRecInside = {
2217
classList: { add: jest.fn(), remove: jest.fn() },
23-
setAttribute: jest.fn()
18+
_recordHandler: null
2419
};
2520

2621
jest.spyOn(document, "getElementById").mockImplementation(id => {
2722
if (id === "record") return mockStart;
28-
if (id === "rec_inside") return mockRecInside;
2923
return null;
3024
});
3125

js/activity/recorder.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ const doRecordButton = activity => {
4242
const setupActivityRecorder = activityInstance => {
4343
activityInstance._doRecordButton = () => {
4444
const that = activityInstance;
45-
const start = document.getElementById("record"),
46-
recInside = document.getElementById("rec_inside");
45+
const start = document.getElementById("record");
4746
let mediaRecorder;
4847
const clickEvent = new Event("click");
4948
let flag = 0;
@@ -182,7 +181,7 @@ const setupActivityRecorder = activityInstance => {
182181
* @param {Blob[]} recordedChunks - The accumulated recorded chunks.
183182
*/
184183
function saveFile(recordedChunks) {
185-
recInside.classList.remove("blink");
184+
start.classList.remove("recording");
186185

187186
const showAlert = message => {
188187
if (window.MBDialog && typeof window.MBDialog.alert === "function") {
@@ -276,7 +275,7 @@ const setupActivityRecorder = activityInstance => {
276275
*/
277276
function createRecorder(stream) {
278277
flag = 1;
279-
recInside.classList.add("blink");
278+
start.classList.add("recording");
280279
that.textMsg(_("Recording started. Click stop to finish."));
281280
start.removeEventListener("click", createRecorder, true);
282281
let recordedChunks = [];
@@ -290,7 +289,6 @@ const setupActivityRecorder = activityInstance => {
290289
mediaRecorder.onstop = function () {
291290
saveFile(recordedChunks);
292291
recordedChunks = [];
293-
recInside.setAttribute("fill", "#ffffff");
294292
};
295293

296294
mediaRecorder.ondataavailable = function (e) {
@@ -326,7 +324,7 @@ const setupActivityRecorder = activityInstance => {
326324
const stopHandler = function stopHandler() {
327325
if (mediaRecorder && mediaRecorder.state === "recording") {
328326
mediaRecorder.stop();
329-
recInside.classList.remove("blink");
327+
start.classList.remove("recording");
330328
flag = 0;
331329
cleanupStreams();
332330
}
@@ -336,7 +334,6 @@ const setupActivityRecorder = activityInstance => {
336334
};
337335
start.addEventListener("click", stopHandler);
338336
}
339-
recInside.setAttribute("fill", "red");
340337
} catch (error) {
341338
ErrorHandler.recoverable(error, { operation: "recording" });
342339
that.textMsg(_("Recording failed: %s").replace(/%s/g, error.message));

js/artwork.js

Lines changed: 1 addition & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

js/toolbar-ui.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
/*
1313
global _THIS_IS_MUSIC_BLOCKS_, docById, doSVG, fnBrowserDetect,
14-
RECORDBUTTON, saveButton, saveButtonAdvanced, ActivityContext
14+
saveButton, saveButtonAdvanced, ActivityContext
1515
*/
1616

1717
/* exported ToolbarUI */
@@ -1049,7 +1049,6 @@ class ToolbarUI {
10491049
Record.classList.remove("hide");
10501050
Record.style.display = "block";
10511051
}
1052-
Record.innerHTML = `<i class="material-icons main">${RECORDBUTTON}</i>`;
10531052

10541053
// Remove any existing onclick handler
10551054
Record.onclick = null;

js/turtledefs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
RHYTHMPALETTEICON, RUNBUTTON, SAVEBUTTON, SCROLLUNLOCKBUTTON,
2121
TABBUTTON_LIGHT, TABBUTTON_DARK, SLOWBUTTON, SMALLERBUTTON, STATSBUTTON,
2222
STEPBUTTON, STOPTURTLEBUTTON, WRAPTURTLEBUTTON, _THIS_IS_TURTLE_BLOCKS_,
23-
_THIS_IS_MUSIC_BLOCKS_, MOUSEPALETTEICON, FULLSCREENBUTTON, RECORDBUTTON,
23+
_THIS_IS_MUSIC_BLOCKS_, MOUSEPALETTEICON, FULLSCREENBUTTON,
2424
PLUGINSBUTTON, OPENMERGEBUTTON, PITCHPREVIEWBUTTON, JAVASCRIPTBUTTON,
2525
RECORDHELPBUTTON, DARKMODEBUTTON, SELECTHELPBUTTON, BLOCKMENUBUTTON,
2626
CANVASMENUBUTTON, RHYTHMPALETTEHELPICON, PITCHPREVIEWHELPBUTTON

0 commit comments

Comments
 (0)