From 14665860331fe6796465391325ec2085a91bfa3f Mon Sep 17 00:00:00 2001 From: Daniel Calviño Sánchez Date: Wed, 18 Apr 2018 14:07:04 +0200 Subject: Fix ids of permission checkboxes for shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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, 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 initial 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 --- core/js/sharedialogshareelistview.js | 22 ++++++++--------- core/js/tests/specs/sharedialogshareelistview.js | 30 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index cda837a66c7..6725f625be4 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -30,8 +30,8 @@ '' + '{{#if editPermissionPossible}}' + '' + - '' + - '' + + '' + + '' + '' + '{{/if}}' + '' + @@ -58,8 +58,8 @@ '{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' + '
  • ' + '' + - '' + - '' + + '' + + '' + '' + '
  • ' + '{{/unless}} {{/if}} {{/if}}' + @@ -67,24 +67,24 @@ '{{#if createPermissionPossible}}{{#unless isMailShare}}' + '
  • ' + '' + - '' + - '' + + '' + + '' + '' + '
  • ' + '{{/unless}}{{/if}}' + '{{#if updatePermissionPossible}}{{#unless isMailShare}}' + '
  • ' + '' + - '' + - '' + + '' + + '' + '' + '
  • ' + '{{/unless}}{{/if}}' + '{{#if deletePermissionPossible}}{{#unless isMailShare}}' + '
  • ' + '' + - '' + - '' + + '' + + '' + '' + '
  • ' + '{{/unless}}{{/if}}' + @@ -383,7 +383,7 @@ var _this = this; this.getShareeList().forEach(function(sharee) { - var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith; + var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareId; checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1"); var $edit = _this.$(checkBoxId); if($edit.length === 1) { diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js index cc0268ba580..8e34225d199 100644 --- a/core/js/tests/specs/sharedialogshareelistview.js +++ b/core/js/tests/specs/sharedialogshareelistview.js @@ -105,6 +105,21 @@ describe('OC.Share.ShareDialogShareeListView', function () { expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true); }); + it('marks edit box as indeterminate when only some permissions are given for sharee with special characters', function () { + shareModel.set('shares', [{ + id: 100, + item_source: 123, + permissions: 1 | OC.PERMISSION_UPDATE, + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user _.@-\'', + share_with_displayname: 'User One', + itemType: 'folder' + }]); + shareModel.set('itemType', 'folder'); + listView.render(); + expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true); + }); + it('Checks edit box when all permissions are given', function () { shareModel.set('shares', [{ id: 100, @@ -119,6 +134,21 @@ describe('OC.Share.ShareDialogShareeListView', function () { listView.render(); expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true); }); + + it('Checks edit box when all permissions are given for sharee with special characters', function () { + shareModel.set('shares', [{ + id: 100, + item_source: 123, + permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE, + share_type: OC.Share.SHARE_TYPE_USER, + share_with: 'user _.@-\'', + share_with_displayname: 'User One', + itemType: 'folder' + }]); + shareModel.set('itemType', 'folder'); + listView.render(); + expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true); + }); }); describe('Manages checkbox events correctly', function () { it('Checks cruds boxes when edit box checked', function () { -- cgit v1.2.3 From f8e3b572c858d68b48ef2ed9ba8efd5d1c6be8a5 Mon Sep 17 00:00:00 2001 From: Daniel Calviño Sánchez Date: Wed, 18 Apr 2018 17:03:45 +0200 Subject: Remove no longer needed escaping of special characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The escaping of special characters was needed when the ids of the permission checkboxes for shares were based on the "shareWith" field. Since they are based on the "shareId" field the escaping is no longer needed, as the "sharedId" is expected to always contain compatible characters. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogshareelistview.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 6725f625be4..1e873a7208e 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -383,9 +383,7 @@ var _this = this; this.getShareeList().forEach(function(sharee) { - var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareId; - checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1"); - var $edit = _this.$(checkBoxId); + var $edit = _this.$('#canEdit-' + _this.cid + '-' + sharee.shareId); if($edit.length === 1) { $edit.prop('checked', sharee.editPermissionState === 'checked'); $edit.prop('indeterminate', sharee.editPermissionState === 'indeterminate'); -- cgit v1.2.3