mirror of
https://github.com/Jermolene/TiddlyWiki5
synced 2026-05-18 19:32:19 +00:00
Update deprecated.js issues (#9833)
* 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
* Move new tests to their own file
* Add backwards-compat regression tests for deprecated.js
Locks in pre-5.4.0 tolerant behaviour of $:/core/modules/utils/
deprecated.js helpers that regressed in PR #9251. Each spec targets an
edge-case input the current one-line modern equivalents reject:
- repeat: negative count / null / undefined str
- startsWith / endsWith: RegExp search arg
- stringifyNumber: null / undefined
- domContains: boolean return, self-check
- hasClass: null element, classless element
- getLocationPath: query preservation, hash stripping
(browser-only; pends in Node because the TW5 sandbox has no `window`)
Also picks up the addClass/removeClass/toggleClass whitespace specs
moved out of test-utils.js by the previous commit, so all deprecated.js
coverage lives together.
Fails 8 specs on current HEAD; the follow-up deprecated.js restoration
commit turns them green.
* Restore pre-5.4.0 behaviour of deprecated.js utilities
PR #9251 replaced several helpers in $:/core/modules/utils/deprecated.js
with one-line ES2017 equivalents that diverge from the originals on
edge-case inputs. Follow-up PRs fixed the most visible cases
(getLocationHash #9622, isDate #9771, addClass empty-string #9561 and
whitespace 005e17537); this commit closes the rest:
- repeat: manual loop tolerates negative count / null / undefined str
- startsWith / endsWith: substring compare tolerates RegExp search arg
- stringifyNumber: `num + ""` coercion tolerates null / undefined
- domContains: `a !== b && a.contains(b)` returns boolean, handles self
- hasClass: null-element guard, strict-false return
- getLocationPath: `toString().split("#")[0]` preserves the query
string in permalinks (startup/story.js:214, 217) -- the most visible
user-facing regression, causing ?lang=de etc. to silently drop.
IE-only fallbacks in Math.sign, strEndsWith, domContains, and
domMatchesSelector are removed; TW5 no longer supports IE.
Covered by the regression specs added in the previous commit.
* make comments more refined
This commit is contained in:
Mario Pietsch
@@ -3,18 +3,43 @@ title: $:/core/modules/utils/deprecated.js
|
||||
type: application/javascript
|
||||
module-type: utils
|
||||
|
||||
Deprecated util functions
|
||||
Deprecated util functions. These preserve the pre-5.4.0 signatures and
|
||||
behaviour for backwards compatibility with plugins and external scripts.
|
||||
Prefer modern alternatives in new code (Array.prototype methods, classList,
|
||||
Math.sign, String.prototype.repeat, etc.).
|
||||
|
||||
\*/
|
||||
|
||||
exports.logTable = (data) => console.table(data);
|
||||
|
||||
exports.repeat = (str,count) => str.repeat(count);
|
||||
/*
|
||||
Repeats a string
|
||||
*/
|
||||
exports.repeat = function(str,count) {
|
||||
var result = "";
|
||||
for(var t=0;t<count;t++) {
|
||||
result += str;
|
||||
}
|
||||
return result;
|
||||
};
|
||||
|
||||
exports.startsWith = (str,search) => str.startsWith(search);
|
||||
/*
|
||||
Check if a string starts with another string
|
||||
*/
|
||||
exports.startsWith = function(str,search) {
|
||||
return str.substring(0, search.length) === search;
|
||||
};
|
||||
|
||||
exports.endsWith = (str,search) => str.endsWith(search);
|
||||
/*
|
||||
Check if a string ends with another string
|
||||
*/
|
||||
exports.endsWith = function(str,search) {
|
||||
return str.substring(str.length - search.length) === search;
|
||||
};
|
||||
|
||||
/*
|
||||
Trim whitespace from the start and end of a string
|
||||
*/
|
||||
exports.trim = function(str) {
|
||||
if(typeof str === "string") {
|
||||
return str.trim();
|
||||
@@ -29,30 +54,54 @@ exports.sign = Math.sign;
|
||||
|
||||
exports.strEndsWith = (str,ending,position) => str.endsWith(ending,position);
|
||||
|
||||
exports.stringifyNumber = (num) => num.toString();
|
||||
exports.stringifyNumber = function(num) {
|
||||
return num + "";
|
||||
};
|
||||
|
||||
// Returns the fully escaped CSS selector for a tag, e.g.
|
||||
// "$:/tags/Stylesheet" -> "tc-tagged-\%24\%3A\%2Ftags\%2FStylesheet"
|
||||
exports.tagToCssSelector = function(tagName) {
|
||||
return "tc-tagged-" + encodeURIComponent(tagName).replace(/[!"#$%&'()*+,\-./:;<=>?@[\\\]^`{\|}~,]/mg,function(c) {
|
||||
return "\\" + c;
|
||||
});
|
||||
};
|
||||
|
||||
exports.domContains = (a,b) => a.compareDocumentPosition(b) & 16;
|
||||
/*
|
||||
Determines whether element 'a' contains element 'b'.
|
||||
Returns false when a === b (matches the original John Resig semantics).
|
||||
*/
|
||||
exports.domContains = function(a,b) {
|
||||
return a !== b && a.contains(b);
|
||||
};
|
||||
|
||||
exports.domMatchesSelector = (node,selector) => node.matches(selector);
|
||||
|
||||
exports.hasClass = (el,className) => el.classList && el.classList.contains(className);
|
||||
exports.hasClass = function(el,className) {
|
||||
return !!(el && el.classList && el.classList.contains(className));
|
||||
};
|
||||
|
||||
// addClass/removeClass/toggleClass split on whitespace to preserve the
|
||||
// original setAttribute("class", ...) acceptance of "foo bar" as two
|
||||
// classes. Regressed in #9251.
|
||||
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;
|
||||
exports.getLocationPath = function() {
|
||||
return window.location.toString().split("#")[0];
|
||||
};
|
||||
|
||||
@@ -0,0 +1,252 @@
|
||||
/*\
|
||||
title: test-deprecated.js
|
||||
type: application/javascript
|
||||
tags: [[$:/tags/test-spec]]
|
||||
|
||||
Regression-guard tests for $:/core/modules/utils/deprecated.js.
|
||||
|
||||
Locks in pre-5.4.0 tolerant behaviour of $tw.utils helpers that regressed
|
||||
in PR #9251 (one-line modern equivalents diverge on edge-case inputs).
|
||||
Without the companion restoration patch to core/modules/utils/deprecated.js:
|
||||
8 specs fail. With the patch: green.
|
||||
|
||||
The addClass/removeClass/toggleClass specs at the end were moved from
|
||||
test-utils.js — RSOD guard for the SampleWizard report (class field
|
||||
"aaa bbb" crashing classList.add with InvalidCharacterError).
|
||||
|
||||
\*/
|
||||
|
||||
"use strict";
|
||||
|
||||
describe("deprecated.js — backwards-compat",function() {
|
||||
|
||||
describe("$tw.utils.repeat",function() {
|
||||
it("returns '' for zero or negative count (does not throw)",function() {
|
||||
expect($tw.utils.repeat("x",0)).toBe("");
|
||||
expect($tw.utils.repeat("x",-1)).toBe("");
|
||||
expect($tw.utils.repeat("x",-100)).toBe("");
|
||||
});
|
||||
it("coerces null/undefined str rather than throwing",function() {
|
||||
expect($tw.utils.repeat(null,3)).toBe("nullnullnull");
|
||||
expect($tw.utils.repeat(undefined,2)).toBe("undefinedundefined");
|
||||
});
|
||||
it("still works for normal inputs",function() {
|
||||
expect($tw.utils.repeat("ab",3)).toBe("ababab");
|
||||
expect($tw.utils.repeat("-",5)).toBe("-----");
|
||||
});
|
||||
});
|
||||
|
||||
describe("$tw.utils.startsWith / $tw.utils.endsWith",function() {
|
||||
it("tolerate a RegExp search argument without throwing",function() {
|
||||
// Old impl uses substring coercion; native String.prototype.startsWith
|
||||
// throws TypeError when passed a RegExp.
|
||||
expect(function() { $tw.utils.startsWith("abc",/a/); }).not.toThrow();
|
||||
expect(function() { $tw.utils.endsWith("abc",/c/); }).not.toThrow();
|
||||
});
|
||||
it("still match normal string inputs",function() {
|
||||
expect($tw.utils.startsWith("abcdef","abc")).toBe(true);
|
||||
expect($tw.utils.startsWith("abcdef","xyz")).toBe(false);
|
||||
expect($tw.utils.endsWith("abcdef","def")).toBe(true);
|
||||
expect($tw.utils.endsWith("abcdef","xyz")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("$tw.utils.stringifyNumber",function() {
|
||||
it("coerces null/undefined via string-concat rather than throwing",function() {
|
||||
expect($tw.utils.stringifyNumber(null)).toBe("null");
|
||||
expect($tw.utils.stringifyNumber(undefined)).toBe("undefined");
|
||||
});
|
||||
it("still returns a number's string form",function() {
|
||||
expect($tw.utils.stringifyNumber(42)).toBe("42");
|
||||
expect($tw.utils.stringifyNumber(-3.14)).toBe("-3.14");
|
||||
expect($tw.utils.stringifyNumber(0)).toBe("0");
|
||||
});
|
||||
});
|
||||
|
||||
describe("$tw.utils.domContains",function() {
|
||||
// Stub nodes expose both .contains() and .compareDocumentPosition() so
|
||||
// both the old (compareDocumentPosition & 16 → number) and new
|
||||
// (a !== b && a.contains(b) → boolean) forms can be observed.
|
||||
function makeNode(children) {
|
||||
children = children || [];
|
||||
var self;
|
||||
self = {
|
||||
contains: function(other) {
|
||||
if(other === self) { return true; }
|
||||
return children.some(function(c) { return c === other || c.contains(other); });
|
||||
},
|
||||
compareDocumentPosition: function(other) {
|
||||
if(other === self) { return 0; }
|
||||
return self.contains(other) ? 16 : 0;
|
||||
}
|
||||
};
|
||||
return self;
|
||||
}
|
||||
it("returns strictly boolean true/false, not a bit-mask number",function() {
|
||||
var child = makeNode();
|
||||
var parent = makeNode([child]);
|
||||
var unrelated = makeNode();
|
||||
expect($tw.utils.domContains(parent,child)).toBe(true);
|
||||
expect($tw.utils.domContains(parent,unrelated)).toBe(false);
|
||||
});
|
||||
it("returns false for domContains(x, x)",function() {
|
||||
var a = makeNode();
|
||||
expect($tw.utils.domContains(a,a)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("$tw.utils.hasClass",function() {
|
||||
it("returns false for null/undefined element without throwing",function() {
|
||||
expect(function() { $tw.utils.hasClass(null,"foo"); }).not.toThrow();
|
||||
expect($tw.utils.hasClass(null,"foo")).toBe(false);
|
||||
expect(function() { $tw.utils.hasClass(undefined,"foo"); }).not.toThrow();
|
||||
expect($tw.utils.hasClass(undefined,"foo")).toBe(false);
|
||||
});
|
||||
it("returns strictly false (not undefined) for elements without classList",function() {
|
||||
expect($tw.utils.hasClass({},"foo")).toBe(false);
|
||||
});
|
||||
it("delegates to classList.contains for real elements",function() {
|
||||
var el = { classList: { contains: function(c) { return c === "a" || c === "b"; } } };
|
||||
expect($tw.utils.hasClass(el,"a")).toBe(true);
|
||||
expect($tw.utils.hasClass(el,"b")).toBe(true);
|
||||
expect($tw.utils.hasClass(el,"c")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// getLocationPath reads window.location: specs pend in Node (no `window`
|
||||
// in the TW5 sandbox) and use history.replaceState in the browser —
|
||||
// assigning to window.location would trigger a navigation and reload.
|
||||
describe("$tw.utils.getLocationPath",function() {
|
||||
var originalUrl;
|
||||
beforeEach(function() {
|
||||
if(!$tw.browser) { return; }
|
||||
originalUrl = window.location.href;
|
||||
});
|
||||
afterEach(function() {
|
||||
if(!$tw.browser) { return; }
|
||||
history.replaceState(null,"",originalUrl);
|
||||
});
|
||||
it("preserves the query string in the returned path",function() {
|
||||
if(!$tw.browser) { pending("browser-only: requires window.location - run in browser"); return; }
|
||||
history.replaceState(null,"","?lang=de&x=1#Intro");
|
||||
var path = $tw.utils.getLocationPath();
|
||||
expect(path).toContain("?lang=de&x=1");
|
||||
expect(path).not.toContain("#Intro");
|
||||
});
|
||||
it("strips the hash fragment",function() {
|
||||
if(!$tw.browser) { pending("browser-only: requires window.location - run in browser"); return; }
|
||||
history.replaceState(null,"","#SomeTiddler");
|
||||
// Sanity check: replaceState actually changed the hash.
|
||||
expect(window.location.hash).toBe("#SomeTiddler");
|
||||
var path = $tw.utils.getLocationPath();
|
||||
expect(path).not.toContain("#");
|
||||
// Rebuild expected href without the hash — works on http(s):// and file://.
|
||||
var expected = window.location.href.split("#")[0];
|
||||
expect(path).toBe(expected);
|
||||
});
|
||||
it("includes the query string when no hash is present",function() {
|
||||
if(!$tw.browser) { pending("browser-only: requires window.location - run in browser"); return; }
|
||||
history.replaceState(null,"","?x=1");
|
||||
var path = $tw.utils.getLocationPath();
|
||||
expect(path).toMatch(/\?x=1$/);
|
||||
expect(path).not.toContain("#");
|
||||
});
|
||||
});
|
||||
|
||||
// Regression guard: classList.add/remove/toggle throw InvalidCharacterError on
|
||||
// whitespace. Manual repro: tw5-com #SampleWizard, class="aaa bbb", Done, popup
|
||||
// -> OK -> nested popup -> RSOD. Stub classList mirrors real DOM semantics
|
||||
// (reject whitespace, de-dupe on add, no-op on remove of 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");
|
||||
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");
|
||||
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);
|
||||
expect(el._tokens).toEqual(["a","b"]);
|
||||
$tw.utils.toggleClass(el,"a b",false);
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
Reference in New Issue
Block a user