Skip to content

Commit 9329ff5

Browse files
committed
XWIKI-22154: "aria-labelledby" for navigation panel entries and corresponding 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
1 parent 6cf8b0d commit 9329ff5

6 files changed

Lines changed: 64 additions & 17 deletions

File tree

xwiki-platform-core/xwiki-platform-attachment/xwiki-platform-attachment-api/src/main/resources/js/attachment/move.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ require(['jquery', config.treeWebjar], function ($) {
7070

7171
selectButton.on('click', function() {
7272
modal.modal('hide');
73-
var docReference = $.jstree.reference(tree).get_selected(false)[0]
73+
var docReference = $.fn.xtree.unescapeNodeId($.jstree.reference(tree).get_selected(false)[0])
7474
const idx = docReference.indexOf(':')
7575
docReference = docReference.substring(idx + 1);
7676
if (docReference) {

xwiki-platform-core/xwiki-platform-tree/xwiki-platform-tree-webjar/src/main/webjar/tree.js

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,32 @@ define([
4848
};
4949
}
5050

51+
// jsTree node identifiers are used, without any escaping, both as the HTML id attribute of the rendered nodes and
52+
// as the value sent back to the server (e.g. to look up children, to move or copy a node to a new parent, etc.).
53+
// Entity references can contain spaces, but HTML ids must not contain any whitespace, so we escape the ids of the
54+
// nodes received from the server before handing them to jsTree, and we unescape them back before sending them to
55+
// the server, in order to keep both usages happy. Other scripts that read a node id from the tree's public API
56+
// (e.g. get_selected()) and need the original, unescaped, entity reference should use $.fn.xtree.unescapeNodeId().
57+
var escapeId = function(id) {
58+
return String(id).replaceAll('%', '%25').replaceAll(' ', '%20');
59+
};
60+
61+
var unescapeId = function(id) {
62+
return String(id).replaceAll('%20', ' ').replaceAll('%25', '%');
63+
};
64+
65+
// Escapes the id of the given node, and of all its nested children (in case the node was received with its
66+
// children already inlined), so that the id can be safely used as an HTML id attribute.
67+
var escapeNodeId = function(node) {
68+
if (node && typeof node.id === 'string') {
69+
node.id = escapeId(node.id);
70+
}
71+
if (node && Array.isArray(node.children)) {
72+
node.children.forEach(escapeNodeId);
73+
}
74+
return node;
75+
};
76+
5177
var formToken = $('meta[name=form_token]').attr('content');
5278

5379
var getNodeTypes = function(nodes) {
@@ -116,11 +142,11 @@ define([
116142
childrenURL = this.element.attr('data-url');
117143
parameters = $.extend({
118144
data: 'children',
119-
id: node.id
145+
id: unescapeId(node.id)
120146
}, parameters);
121147
}
122148
if (childrenURL) {
123-
post(childrenURL, parameters).then(callback, () => callback([]));
149+
post(childrenURL, parameters).then(children => children.map(escapeNodeId)).then(callback, () => callback([]));
124150
} else {
125151
callback([]);
126152
}
@@ -363,7 +389,8 @@ define([
363389
// We need to retrieve the node path from the server.
364390
var url = this.element.attr('data-url');
365391
if (url) {
366-
return Promise.resolve(post(url, {data: 'path', 'id': nodeId}));
392+
return Promise.resolve(
393+
post(url, {data: 'path', 'id': unescapeId(nodeId)}).then(nodes => nodes.map(escapeNodeId)));
367394
} else {
368395
return Promise.reject();
369396
}
@@ -519,7 +546,11 @@ define([
519546
if (!url) {
520547
url = this.element.attr('data-url');
521548
params.action = action;
522-
params.id = node.id;
549+
params.id = unescapeId(node.id);
550+
}
551+
if (params.parent) {
552+
// The parent is specified, as a node id, when moving or copying a node to a new parent.
553+
params.parent = unescapeId(params.parent);
523554
}
524555
params.form_token = formToken;
525556
var promise = this.jobRunner.run(url, params);
@@ -759,5 +790,15 @@ define([
759790
});
760791
};
761792

793+
// Turns a node id read from the tree's public API (e.g. get_selected()) back into the original, unescaped, entity
794+
// reference. Node ids are escaped so they can be safely used as HTML id attributes (see #escapeNodeId), so scripts
795+
// that need to resolve a node id into an entity reference, rather than pass it back to the tree itself, must
796+
// unescape it first.
797+
$.fn.xtree.unescapeNodeId = unescapeId;
798+
799+
// Escapes the id of a node built outside of this module (e.g. the response of a job triggered through #execute)
800+
// before inserting it into the tree, so that the tree's ids stay consistently escaped.
801+
$.fn.xtree.escapeNodeId = escapeNodeId;
802+
762803
return $;
763804
});

xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/uicomponents/job/question/ExtensionBreakingQuestion.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ require(['jquery', 'xwiki-meta', 'xwiki-tree'], function($, xm) {
4444
for (var i = 0; i < selectedNodes.length; ++i) {
4545
var node = selectedNodes[i];
4646
if (node.data.type == 'extension') {
47-
answerProperties.selectedExtensions.push(node.id);
47+
answerProperties.selectedExtensions.push($.fn.xtree.unescapeNodeId(node.id));
4848
} else if (node.data.type == 'page') {
49-
answerProperties.selectedDocuments.push(node.id);
49+
answerProperties.selectedDocuments.push($.fn.xtree.unescapeNodeId(node.id));
5050
} else if (node.id == 'freePages') {
5151
// For free pages, we can rely on the state of the "freePage" node
5252
answerProperties.selectAllFreePages = true;

xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/uicomponents/job/question/XClassBreakingQuestion.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ require(['jquery', 'xwiki-meta', 'xwiki-tree'], function($, xm) {
4646
// For free pages, we can rely on the state of the "freePage" node
4747
answerProperties.selectAllFreePages = true;
4848
} else {
49-
answerProperties.selectedDocuments.push(node.id);
49+
answerProperties.selectedDocuments.push($.fn.xtree.unescapeNodeId(node.id));
5050
}
5151
}
5252

xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/uicomponents/widgets/locationPicker.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ require(['xwiki-tree'], function($) {
8787
});
8888

8989
// Document Tree Picker
90-
require(['jquery', 'xwiki-meta'], function($, xm) {
90+
require(['jquery', 'xwiki-meta', 'xwiki-tree'], function($, xm) {
9191
$('.location-picker').each(function() {
9292
var picker = $(this);
9393
// The wiki field can be either a select (drop down) or an input (text or hidden).
@@ -109,7 +109,7 @@ require(['jquery', 'xwiki-meta'], function($, xm) {
109109
});
110110

111111
picker.find('.modal').on('xwiki:locationTreePicker:select', function(event, data) {
112-
var selectedNodeId = data.tree.get_selected()[0];
112+
var selectedNodeId = $.fn.xtree.unescapeNodeId(data.tree.get_selected()[0]);
113113
var separatorIndex = selectedNodeId.indexOf(':');
114114
var nodeType = selectedNodeId.substr(0, separatorIndex);
115115
var nodeStringReference = selectedNodeId.substr(separatorIndex + 1);

xwiki-platform-core/xwiki-platform-wysiwyg/xwiki-platform-wysiwyg-webjar/src/main/webjar/resource/entityResourcePicker.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,17 @@ define('xwiki-wysiwyg-entity-resource-picker', [
9898
}).on('xtree.runJob', function (event, promise, action, node, params) {
9999
if (action === 'create') {
100100
promise.then(function (promiseData) {
101-
if (createdNodes[params.id] === undefined) {
102-
createdNodes[params.id] = {};
101+
// The parent node id is already escaped (it comes from the tree's own model), but params.id was
102+
// unescaped by #execute() before being sent to the server, so we use the parent node id instead in
103+
// order to match what #refresh_node.jstree reads later from data.node.id.
104+
if (createdNodes[node.id] === undefined) {
105+
createdNodes[node.id] = {};
103106
}
104107
if (promiseData instanceof Array) {
105-
let newNode = promiseData[0];
106-
createdNodes[params.id][newNode.id] = newNode;
108+
// The job response carries the raw, unescaped, id of the newly created node, so we need to escape it
109+
// before it can be inserted into the tree.
110+
let newNode = $.fn.xtree.escapeNodeId(promiseData[0]);
111+
createdNodes[node.id][newNode.id] = newNode;
107112
}
108113
});
109114
}
@@ -159,9 +164,10 @@ define('xwiki-wysiwyg-entity-resource-picker', [
159164
};
160165

161166
var getEntityReference = function(node) {
162-
var separatorIndex = node.id.indexOf(':');
163-
var nodeType = node.id.substr(0, separatorIndex);
164-
var nodeStringReference = node.id.substr(separatorIndex + 1);
167+
var nodeId = $.fn.xtree.unescapeNodeId(node.id);
168+
var separatorIndex = nodeId.indexOf(':');
169+
var nodeType = nodeId.substr(0, separatorIndex);
170+
var nodeStringReference = nodeId.substr(separatorIndex + 1);
165171
return XWiki.Model.resolve(nodeStringReference, XWiki.EntityType.byName(nodeType));
166172
};
167173

0 commit comments

Comments
 (0)