Disclaimer up front: I don’t know this topic well, so I asked Claude Code to dig into the PR and the tree code. Below is what it came back with. It looks interesting enough to be worth posting, but please take it as input to be double-checked rather than as my own reviewed opinion.
The escaping in #6205 is entirely client-side, so the wire format is unchanged. tree.js unescapes before every server call and escapes what comes back, which means anything talking to the tree data URL directly is unaffected – including Cristal: its componentsInit.ts fetches bin/get?sheet=XWiki.DocumentTree&data=children&id=document:xwiki:... itself and reads treeNode.id from the JSON response, never touching the jsTree model. Same for the {{tree}}/{{documentTree}} macro’s openTo and root parameters. So @pjeanjean there should be nothing to change on the Cristal side, and no need to detect the id format per instance. The break is narrower than the proposal suggests: only in-page JS that reads an id out of the jsTree model (get_selected(), data.node.id) and parses it as a reference.
But the PR only handles the outbound direction (tree → id); the inbound direction (id → tree) still builds raw ids and feeds them into the tree. Places that look like they would still be broken for a name containing a space: exporter.js:570-573 (builds 'document:' + serialize(currentDocument) then open_node/select_node – it is listed as safe in the PR, and open_node on an unknown id never fires its callback, so the promise never resolves); PanelsCode/NavigationConfigurationSheet.xml:369 (creates a node with a raw id, so the invalid HTML id survives there), plus :413 get_node(...) and :480 hide_node(...); XWiki/InplaceEditing.xml:1266 set_text('document:' + ...); and tree_macros.vm:108/:113, where the macro’s data/rootData options go into data-json/data-root and bypass escapeNodeId() entirely.
Suggested change of invariant: escape on the way in, unescape only on the way out. Right now unescapeId() is applied both to ids coming from the tree model (correct) and to caller-supplied raw ids in getPath()/openTo() (locationPicker.js, attachment/move.js, AnnotationConfigSheet.xml), where it silently turns a page literally named A%20B into A B – the same class of bug as XWIKI-18356/XWIKI-22151. Escaping every entry point instead (openTo, data-openTo, data-json, data-root, job responses) fixes that, fixes the inline-JSON case, and keeps all existing raw-id inputs working, which shrinks the compatibility break down to just “reading ids out of jsTree” – exactly where unescapeNodeId() belongs.
Two smaller things. The escaping only covers space and %, but HTML5 forbids all ASCII whitespace in id (tab, LF, CR, FF too); and since the format is breaking anyway, type: + encodeURIComponent(reference) would be a standard, self-documenting scheme that also escapes the \ which already breaks By.id() in Selenium (see the comment in TreeElement.hasNode). And given this has already regressed twice (#3015 reverted → XWIKI-22151 → XWIKI-22154), a functional test that moves a page into a space literally named A B and asserts no whitespace in the rendered id/aria-activedescendant seems like the thing that makes it stick.
On the actual question: +1 for the break, and the line that seems defensible is “the wire/macro-level id format stays public, the escaped client-side form is an implementation detail of the widget, with escapeNodeId()/unescapeNodeId() as the documented bridge” – which is what the code already does.