Fix ids of permission checkboxes for shares
The ids of permission checkboxes for shares were generated using the "shareWith" field of the share. The "shareWith" field can contain spaces (as spaces are allowed, for example, in user or circle names), so this could cause the id attribute of the HTML element to contain spaces too, which is forbidden by the HTML specification. It is not just a "formal" issue, though; when the list was rendered after a permission change, if the id contained a space the selector to get the checkbox element was wrong (as it ended being something like "#canEdit-view1-name with spaces") and thus the updated state of the checkbox was not properly set. Besides that, "shareWith" can contain too single quotes, which would even cause the jQuery selector to abort the search and leave the UI in an invalid state. Instead of adding more cases to the regular expression to escape special characters and apply it too when the ids are created now the ids of permission checkboxes for shares are based on the "shareId" field instead of on "shareWith", as "shareId" is expected to always contain compatible characters. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
parent
3a013b127c
commit
0734325dda
|
@ -30,8 +30,8 @@
|
|||
'<span class="sharingOptionsGroup">' +
|
||||
'{{#if editPermissionPossible}}' +
|
||||
'<span class="shareOption">' +
|
||||
'<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" {{#if hasEditPermission}}checked="checked"{{/if}} />' +
|
||||
'<label for="canEdit-{{cid}}-{{shareWith}}">{{canEditLabel}}</label>' +
|
||||
'<input id="canEdit-{{cid}}-{{shareId}}" type="checkbox" name="edit" class="permissions checkbox" {{#if hasEditPermission}}checked="checked"{{/if}} />' +
|
||||
'<label for="canEdit-{{cid}}-{{shareId}}">{{canEditLabel}}</label>' +
|
||||
'</span>' +
|
||||
'{{/if}}' +
|
||||
'<a href="#"><span class="icon icon-more"></span></a>' +
|
||||
|
@ -58,8 +58,8 @@
|
|||
'{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' +
|
||||
'<li>' +
|
||||
'<span class="shareOption menuitem">' +
|
||||
'<input id="canShare-{{cid}}-{{shareWith}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' +
|
||||
'<label for="canShare-{{cid}}-{{shareWith}}">{{canShareLabel}}</label>' +
|
||||
'<input id="canShare-{{cid}}-{{shareId}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' +
|
||||
'<label for="canShare-{{cid}}-{{shareId}}">{{canShareLabel}}</label>' +
|
||||
'</span>' +
|
||||
'</li>' +
|
||||
'{{/unless}} {{/if}} {{/if}}' +
|
||||
|
@ -67,24 +67,24 @@
|
|||
'{{#if createPermissionPossible}}{{#unless isMailShare}}' +
|
||||
'<li>' +
|
||||
'<span class="shareOption menuitem">' +
|
||||
'<input id="canCreate-{{cid}}-{{shareWith}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' +
|
||||
'<label for="canCreate-{{cid}}-{{shareWith}}">{{createPermissionLabel}}</label>' +
|
||||
'<input id="canCreate-{{cid}}-{{shareId}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' +
|
||||
'<label for="canCreate-{{cid}}-{{shareId}}">{{createPermissionLabel}}</label>' +
|
||||
'</span>' +
|
||||
'</li>' +
|
||||
'{{/unless}}{{/if}}' +
|
||||
'{{#if updatePermissionPossible}}{{#unless isMailShare}}' +
|
||||
'<li>' +
|
||||
'<span class="shareOption menuitem">' +
|
||||
'<input id="canUpdate-{{cid}}-{{shareWith}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' +
|
||||
'<label for="canUpdate-{{cid}}-{{shareWith}}">{{updatePermissionLabel}}</label>' +
|
||||
'<input id="canUpdate-{{cid}}-{{shareId}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' +
|
||||
'<label for="canUpdate-{{cid}}-{{shareId}}">{{updatePermissionLabel}}</label>' +
|
||||
'</span>' +
|
||||
'</li>' +
|
||||
'{{/unless}}{{/if}}' +
|
||||
'{{#if deletePermissionPossible}}{{#unless isMailShare}}' +
|
||||
'<li>' +
|
||||
'<span class="shareOption menuitem">' +
|
||||
'<input id="canDelete-{{cid}}-{{shareWith}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' +
|
||||
'<label for="canDelete-{{cid}}-{{shareWith}}">{{deletePermissionLabel}}</label>' +
|
||||
'<input id="canDelete-{{cid}}-{{shareId}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' +
|
||||
'<label for="canDelete-{{cid}}-{{shareId}}">{{deletePermissionLabel}}</label>' +
|
||||
'</span>' +
|
||||
'</li>' +
|
||||
'{{/unless}}{{/if}}' +
|
||||
|
@ -380,7 +380,7 @@
|
|||
var $li = this.$('li[data-share-id=' + permissionChangeShareId + ']');
|
||||
$li.find('.sharingOptionsGroup .popovermenu').replaceWith(this.popoverMenuTemplate(sharee));
|
||||
|
||||
var checkBoxId = 'canEdit-' + this.cid + '-' + sharee.shareWith;
|
||||
var checkBoxId = 'canEdit-' + this.cid + '-' + sharee.shareId;
|
||||
checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1");
|
||||
var $edit = $li.parent().find(checkBoxId);
|
||||
if($edit.length === 1) {
|
||||
|
|
|
@ -122,6 +122,56 @@ describe('OC.Share.ShareDialogShareeListView', function () {
|
|||
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
|
||||
expect(updateShareStub.calledOnce).toEqual(true);
|
||||
});
|
||||
|
||||
it('Checks edit box when clicking on it', function () {
|
||||
updateShareStub.callsFake(function (shareId, attributes) {
|
||||
// Update the permissions to get the new value when rendering
|
||||
// again.
|
||||
var shareIndex = this.findShareWithIndex(shareId);
|
||||
this.get('shares')[shareIndex].permissions = attributes.permissions;
|
||||
});
|
||||
|
||||
shareModel.set('shares', [{
|
||||
id: 100,
|
||||
item_source: 123,
|
||||
permissions: 0,
|
||||
share_type: OC.Share.SHARE_TYPE_USER,
|
||||
share_with: 'user1',
|
||||
share_with_displayname: 'User One'
|
||||
}]);
|
||||
shareModel.set('itemType', 'folder');
|
||||
listView.render();
|
||||
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(false);
|
||||
listView.$el.find("input[name='edit']").click();
|
||||
listView.render();
|
||||
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
|
||||
expect(updateShareStub.calledOnce).toEqual(true);
|
||||
});
|
||||
|
||||
it('Checks edit box when clicking on it for sharee with special characters', function () {
|
||||
updateShareStub.callsFake(function (shareId, attributes) {
|
||||
// Update the permissions to get the new value when rendering
|
||||
// again.
|
||||
var shareIndex = this.findShareWithIndex(shareId);
|
||||
this.get('shares')[shareIndex].permissions = attributes.permissions;
|
||||
});
|
||||
|
||||
shareModel.set('shares', [{
|
||||
id: 100,
|
||||
item_source: 123,
|
||||
permissions: 0,
|
||||
share_type: OC.Share.SHARE_TYPE_USER,
|
||||
share_with: 'user _.@-\'',
|
||||
share_with_displayname: 'User One'
|
||||
}]);
|
||||
shareModel.set('itemType', 'folder');
|
||||
listView.render();
|
||||
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(false);
|
||||
listView.$el.find("input[name='edit']").click();
|
||||
listView.render();
|
||||
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
|
||||
expect(updateShareStub.calledOnce).toEqual(true);
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue