Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable13] Fix ids of permission checkboxes for shares #9453

Merged

Conversation

danxuliu
Copy link
Member

Backport of #9237

Note that as #8390 was not backported the issues in stable13 are not exactly the same as the ones described in #9237; in master the problems appear as soon as the shares list is rendered, while in stable13 they appear only when the share permissions are updated, for example, when clicking on the Can edit checkbox.

danxuliu added 2 commits May 10, 2018 21:39
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 <[email protected]>
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 <[email protected]>
@codecov
Copy link

codecov bot commented May 10, 2018

Codecov Report

Merging #9453 into stable13 will increase coverage by 0.01%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             stable13    #9453      +/-   ##
==============================================
+ Coverage       51.36%   51.38%   +0.01%     
  Complexity      25051    25051              
==============================================
  Files            1609     1609              
  Lines           95348    95346       -2     
  Branches         1376     1376              
==============================================
+ Hits            48974    48991      +17     
+ Misses          46374    46355      -19
Impacted Files Coverage Δ Complexity Δ
core/js/sharedialogshareelistview.js 48.75% <100%> (+3.52%) 0 <0> (ø) ⬇️
lib/private/Files/Cache/Propagator.php 96.2% <0%> (+1.26%) 16% <0%> (ø) ⬇️
core/js/shareitemmodel.js 89.4% <0%> (+2.18%) 0% <0%> (ø) ⬇️

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

@MorrisJobke MorrisJobke merged commit e3d694f into stable13 May 14, 2018
@MorrisJobke MorrisJobke deleted the stable13-9237-fix-ids-of-permission-checkboxes-for-shares branch May 14, 2018 11:41
@MorrisJobke MorrisJobke mentioned this pull request May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants