XWIKI-22154: "aria-labelledby" for navigation panel entries and corresponding ids contain illegal whitespaces - #6205
XWIKI-22154: "aria-labelledby" for navigation panel entries and corresponding ids contain illegal whitespaces#6205Sereza5 wants to merge 2 commits into
Conversation
|
Will keep this PR as a draft until the discussion at https://forum.xwiki.org/t/escaping-jstree-node-ids/18767 is solved :) |
|
@Sereza5 FYI, I have posted a comment (CC did most of the work) at https://forum.xwiki.org/t/escaping-jstree-node-ids/18767/3 Please check if it makes sense.Thx. |
9329ff5 to
3cb9f56
Compare
|
Reworked in 3cb9f56 following the forum analysis: node ids are now escaped on the way in (server responses, data-json, openTo()) and unescaped only on the way out (ids sent to the server, tree field value). This fixes the open_node/select_node/get_node/hide_node/set_text calls that build a raw id, the data-json case, and getPath()/openTo() wrongly unescaping caller-supplied ids. Existing raw-id inputs keep working, so the break is limited to reading an id out of jsTree and parsing it as a reference. Rebased on master, so Two functional tests added: one asserting the tree renders no whitespace in id/aria-activedescendant/aria-labelledby for a page under a space named A B, and one moving a page into a space named Rename Space through the tree picker (the XWIKI-22151 regression). I ran a |
…sponding ids contain illegal whitespaces * Escaped jsTree node ids when received from the server and unescaped them when sent back * Added $.fn.xtree.unescapeNodeId() and $.fn.xtree.escapeNodeId() helpers * Unescaped the selected node id in the location picker * Unescaped the selected node id when moving an attachment * Unescaped and escaped node ids in the WYSIWYG entity resource picker * Unescaped node ids collected in the extension and class breaking question dialogs
…sponding ids contain illegal whitespaces * Escaped the node ids on the way into the tree and unescaped them only on the way out * Percent-encoded the node ids, keeping the colon and the at sign as is * Escaped the node ids passed to openTo() and the ones specified inline through data-json * Unescaped the selected node ids written into the tree field value * Escaped the node ids built by the export tree, the navigation panel configuration, the in-place editor and the annotation configuration * Unescaped the selected node ids in the template provider picker and the WYSIWYG image selector * Escaped and unescaped the node ids in the tree page objects * Added a functional test for the escaping of the node ids * Added a functional test for moving a page to a parent whose name contains white space
2 participants
Jira URL
https://jira.xwiki.org/browse/XWIKI-22154
Changes
Description
%) before handing them to jsTree, and unescape them back before sending them to the server, so that the renderedid/anchorid/aria-activedescendantattributes never contain literal whitespace.$.fn.xtree.unescapeNodeId()and$.fn.xtree.escapeNodeId()so other scripts can convert between the tree's escaped node id and the raw entity reference.get_selected(),data.node.id) and resolves it into an entity reference, to unescape it first: the location picker (page rename/move), the attachment move dialog, the WYSIWYG entity resource picker, and the extension/class "breaking pages" delete-confirmation dialogs.Clarifications
getChildren, which missed the "move to a new parent" case and broke XWIKI-22151 (moving a page into a space with a space in its name created a literalA%20Bspace). It was reverted.tree.jsfor every id the tree itself sends to or receives from the server (children, path, the acted-on node and the move/copy destination parent), and every other consumer that independently parses a tree node id back into an entity reference has been updated to unescape it first, so the same class of regression shouldn't resurface.get_selected()/readingdata.node.idand checking whether they parse the id into an entity reference (risky) or just use it as an opaque token / literal comparison (safe, left untouched — e.g.exporter.jsand thedata.idfield already carry the bare, unprefixed, unescaped reference and are unaffected).Compatibility note: the jsTree node
id(type:reference) is not a documented/@Unstable-tagged API and isn't covered by Revapi (it's JS-only), but it is de facto relied on: this PR itself had to update 6 in-repo scripts that parse it as an entity reference. Any xwiki-contrib extension or wiki-page script doing the same thing (readingget_selected()/data.node.idand parsing it) would need the same one-line fix, now available as$.fn.xtree.unescapeNodeId(). I couldn't find a way to avoid this: jsTree hard-codes the renderedidattribute to the node's model id and resolves clicks back to a model node by reading that same DOM attribute, so a valid (whitespace-free) HTML id and a raw, unescaped model id are mutually exclusive with jsTree's current internals — I verified this by prototyping the DOM-only alternative and it broke click-to-select. The original XWIKI-18356 PR (#3015) took the same position on this id not being a sanctioned front-end API; happy to raise it on the dev list instead if reviewers disagree.Screenshots & Video
No visible UI change; the fix only affects the
id/aria-activedescendantattribute values (now percent-escaped instead of containing raw whitespace) and internal id plumbing.Executed Tests
mvn clean install -B -ntp -Plegacy -pl xwiki-platform-core/xwiki-platform-tree/xwiki-platform-tree-webjar,xwiki-platform-core/xwiki-platform-attachment/xwiki-platform-attachment-api,xwiki-platform-core/xwiki-platform-wysiwyg/xwiki-platform-wysiwyg-webjar— builds clean, jshint/Checkstyle/Revapi pass.mvn clean install -B -ntp -Plegacy -pl xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war— builds clean, jshint/Checkstyle/Revapi pass.id, anchoridandaria-activedescendantfor a page/space with a space in the name are all percent-escaped, no literal whitespace.A Bvia the rename/move tree picker: the destination breadcrumb correctly shows "A B" and the page actually lands in theA Bspace (verified via REST), not in a corruptedA%20Bspace — i.e. the XWIKI-22151 regression does not reproduce.Expected merging strategy