From 005e17537867bb88717fa9ac29dcb41ee5f90ba3 Mon Sep 17 00:00:00 2001 From: pmario Date: Wed, 22 Apr 2026 17:15:35 +0200 Subject: [PATCH] Fix RSOD when $tw.utils.addClass receives a class string with whitespace PR #9251 replaced the manual setAttribute("class", ...) implementation of $tw.utils.addClass/removeClass/toggleClass with direct Element.classList calls. Unlike setAttribute, classList.add/remove/toggle throws InvalidCharacterError on any token containing whitespace, so callers that pass a whole class string (e.g. modal.js passing tiddler.fields.class) now crash. Manual repro on tw5-com: open SampleWizard, set the `class` field to "aaa bbb", Done, open popup -> OK -> open nested popup -> RSOD. Fix: split the className argument on whitespace in deprecated.js and feed individual tokens to classList. A small splitClasses() helper keeps the three functions symmetrical. Adds adversarial regression tests in test-utils.js covering: - ASCII whitespace variants (space, tab, CR, LF, mixed runs, padding) - Unicode whitespace (U+00A0 non-breaking space) - de-duplication across single and multiple calls - remove/toggle no-op on missing tokens - toggle with status undefined / true / false - silent no-op for whitespace-only / empty / non-string / null input - silent no-op when the element has no classList --- core/modules/utils/deprecated.js | 14 +++- editions/test/tiddlers/tests/test-utils.js | 98 ++++++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/core/modules/utils/deprecated.js b/core/modules/utils/deprecated.js index a44c3fe962..5d0eb6e937 100644 --- a/core/modules/utils/deprecated.js +++ b/core/modules/utils/deprecated.js @@ -43,16 +43,24 @@ exports.domMatchesSelector = (node,selector) => node.matches(selector); exports.hasClass = (el,className) => el.classList && el.classList.contains(className); +// classList.add/remove/toggle reject whitespace, but the legacy API accepts "aaa bbb". +function splitClasses(className) { + return (typeof className === "string" && className.match(/\S+/g)) || []; +} + exports.addClass = function(el,className) { - el.classList && className && el.classList.add(className); + if(!el.classList) return; + splitClasses(className).forEach(function(c) { el.classList.add(c); }); }; exports.removeClass = function(el,className) { - el.classList && className && el.classList.remove(className); + if(!el.classList) return; + splitClasses(className).forEach(function(c) { el.classList.remove(c); }); }; exports.toggleClass = function(el,className,status) { - el.classList && className && el.classList.toggle(className, status); + if(!el.classList) return; + splitClasses(className).forEach(function(c) { el.classList.toggle(c,status); }); }; exports.getLocationPath = () => window.location.origin + window.location.pathname; \ No newline at end of file diff --git a/editions/test/tiddlers/tests/test-utils.js b/editions/test/tiddlers/tests/test-utils.js index d0b7d96fe6..8d1c8f8b8d 100644 --- a/editions/test/tiddlers/tests/test-utils.js +++ b/editions/test/tiddlers/tests/test-utils.js @@ -206,4 +206,102 @@ describe("Utility tests", function() { expect($tw.utils.insertSortedArray(["b","c","d"],"ccc").join(",")).toEqual("b,c,ccc,d"); }); + // Regression guard #9831: + // classList.add/remove/toggle throw InvalidCharacterError on whitespace. + // Manual repro: open tw5-com #SampleWizard, set `class` field to "aaa bbb", Done, + // open the popup -> OK -> open nested popup -> RSOD without this fix. + // Stubbed classList mimics the real DOM: it rejects any whitespace in a token, + // de-duplicates on add, and no-ops on remove of a missing token. + describe("addClass/removeClass/toggleClass",function() { + function makeEl() { + var tokens = []; + function reject(t) { if(/\s/.test(t)) { throw new Error("InvalidCharacterError: '" + t + "'"); } } + return { + classList: { + add: function() { + for(var i = 0; i < arguments.length; i++) { + reject(arguments[i]); + if(tokens.indexOf(arguments[i]) === -1) { tokens.push(arguments[i]); } + } + }, + remove: function() { + for(var i = 0; i < arguments.length; i++) { + reject(arguments[i]); + var idx = tokens.indexOf(arguments[i]); + if(idx !== -1) { tokens.splice(idx,1); } + } + }, + toggle: function(cls,status) { + reject(cls); + var has = tokens.indexOf(cls) !== -1; + var want = status === undefined ? !has : status; + if(want && !has) { tokens.push(cls); } + if(!want && has) { tokens.splice(tokens.indexOf(cls),1); } + } + }, + _tokens: tokens + }; + } + + it("splits on every ASCII-whitespace flavour (space, tab, newline, CR, mixed runs, leading/trailing)",function() { + var el = makeEl(); + $tw.utils.addClass(el," a\tb\nc\r\nd \t e "); + expect(el._tokens).toEqual(["a","b","c","d","e"]); + }); + + it("splits on Unicode whitespace too (U+00A0 non-breaking space, a common paste-in hazard)",function() { + var el = makeEl(); + $tw.utils.addClass(el,"a\u00A0b"); + expect(el._tokens).toEqual(["a","b"]); + }); + + it("de-duplicates tokens within one call and across calls",function() { + var el = makeEl(); + $tw.utils.addClass(el,"x x y"); + $tw.utils.addClass(el,"y z"); + expect(el._tokens).toEqual(["x","y","z"]); + }); + + it("remove is a no-op for missing tokens and tolerates mixed-presence input",function() { + var el = makeEl(); + $tw.utils.addClass(el,"a b"); + $tw.utils.removeClass(el,"b c d"); // only b is present + expect(el._tokens).toEqual(["a"]); + }); + + it("toggle with no status flips each token independently",function() { + var el = makeEl(); + $tw.utils.addClass(el,"a"); + $tw.utils.toggleClass(el,"a b"); // remove a, add b + expect(el._tokens).toEqual(["b"]); + }); + + it("toggle with status=true/false forces state regardless of current",function() { + var el = makeEl(); + $tw.utils.addClass(el,"a"); + $tw.utils.toggleClass(el,"a b",true); // both on + expect(el._tokens).toEqual(["a","b"]); + $tw.utils.toggleClass(el,"a b",false); // both off + expect(el._tokens).toEqual([]); + }); + + it("is a silent no-op for whitespace-only / empty / non-string / null / undefined className",function() { + var el = makeEl(); + var inputs = ["", " \t\n ", null, undefined, 42, {}, ["a"]]; + inputs.forEach(function(v) { + expect(function() { $tw.utils.addClass(el,v); }).not.toThrow(); + expect(function() { $tw.utils.removeClass(el,v); }).not.toThrow(); + expect(function() { $tw.utils.toggleClass(el,v); }).not.toThrow(); + }); + expect(el._tokens).toEqual([]); + }); + + it("is a silent no-op when element has no classList (SVG in old browsers, detached nodes, stubs)",function() { + var el = {}; + expect(function() { $tw.utils.addClass(el,"a b"); }).not.toThrow(); + expect(function() { $tw.utils.removeClass(el,"a b"); }).not.toThrow(); + expect(function() { $tw.utils.toggleClass(el,"a b",true); }).not.toThrow(); + }); + }); + });