In the environment section of the upgrade status report, next to "Invalid permissions will trigger runtime exceptions in Drupal 10.", the list of invalid permissions contains quotes that appear to have been escaped twice so they appear as HTML entities. Example:
"perm1", "perm1" of user role: "Role Name".
This might be happening when it is run through t().
It might be easier to read if the permissions were presented in an HTML unordered list instead of comma-separated. That would remove the need for quote marks.
The code that generates this is in src/Form/UpgradeStatusForm.php
.
Comment | File | Size | Author |
---|---|---|---|
#12 | Screen Shot 2022-01-27 at 3.41.57 PM.png | 119.83 KB | pmagunia |
#9 | Screen Shot 2022-01-27 at 10.25.57 AM.png | 104.88 KB | pmagunia |
#5 | Screen Shot 2022-01-26 at 5.53.59 PM.png | 83.39 KB | pmagunia |
Issue fork upgrade_status-3260723
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
pmaguniaNot sure this is the right way to do it.
The brackets in
<li></li>
were always appearing as HTML entities no matter what I did.Feedback is welcome of course.
Comment #5
pmaguniaHere's a screenshot of what the output looks like with my code.
Comment #6
Gábor HojtsyGood find!
We know the permission names are code provided and should not have XSS, so we can also just change it to
!permissions
from@permissions
and thus avoid the escaping. I think the quotes make it easier to identify which permission is which. Some permissions are only slightly different from others.Comment #7
Liam MorlandIf the permissions are in a comma-separated list, they should have quotes.
I think it would be even easier to read as an unordered list. That could be generated by theming it as an item list.
Comment #8
Gábor HojtsyItemlist sounds like a good idea as well to make it readable.
Comment #9
pmaguniaI was able to get permissions in quotes but wasn't able to get them in list item brackets.
Any HTML tags I add are not being rendered.
I tried using
!permissions
but then the substitution was not being made.Attaching a screenshot of what my latest MR displays.
Comment #10
Liam MorlandAll that is needed is the use of
#markup
. This commit removes everything except that.Placeholders that start with
!
are no longer supported.Comment #11
Liam MorlandWith this commit, it uses an unordered list. It wasn't working earlier because the data needs to be set as
#markup
.This doesn't use a theme function, which seems heavy-handed for such a simple use case.
I have noticed that only the last role with invalid permissions is displayed. I plan to open a separate ticket to fix that once this one is done.
Comment #12
pmaguniaAh I see. The markup looks much better now.
I can confirm the unordered list is being displayed correctly.
Comment #14
Gábor HojtsyThanks, this looks fine, committed.
Comment #16
Gábor HojtsyWhoops, this actually broke tests, which I noticed in unrelated issues. Tests need to be updated to still pass with the new order of information.
Comment #17
Gábor HojtsyI tried to figure out on my own how to add testing to this MR, since the project aleady has testing, but there is no apparent way. The docs also says that issues on projects with testing configured should get testing for MRs. So not sure how to make that appear. If this is an MR bug we cannot work around, we can switch to patches for this issue.
Comment #18
Gábor HojtsySo turns out my issue testing default configuration for DrupalCI got disabled earlier. I picked another one and that should be fine for new issues I hope, let's try moving to needs review if that triggers the testbot.
Comment #19
Liam MorlandTests updated and now pass.
Comment #21
Gábor HojtsyLooks good, thanks! Committed this one!
Hah, good find! Please do!
Comment #22
Liam Morland#3262102: Only the last role with invalid permissions is displayed