Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to reproduce and test
- Fresh Drupal install (with either Seven or Claro as admin theme)
- Disable JavaScript aggregation in "admin/config/development/performance", and clear cache.
- Go to "admin/people/permissions"
- Check the network tab -> misc/checkbox.js won't be loaded
- Confirm the above by ticking any unticked permission checkbox for the "Authenticated" role:
- actual behavior: the respective checkboxes for the permissions for all other roles disappear, except for the Anonymous role
- expected behavior: the respective checkboxes for the permissions should be ticked for all other roles, except Anonymous
- Apply the patch, clear cache and check the network tab again -> misc/checkbox.js should be loaded.
- Try clicking any unticked checkbox for the "Authenticated" role -> the respective checkboxes for the permissions should be ticked for all other roles, except Anonymous
In fact, we need to register the lib explicitly: (above reproduce steps isn't important)
https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/modules/user...
This change may improve/reduce the performance: #3244564: Improve performance of /admin/people/permissions
Comment | File | Size | Author |
---|---|---|---|
#7 | 3244737-8.patch | 2.85 KB | longwave |
| |||
#7 | 3244737-8-test-only.patch | 2.46 KB | longwave |
#2 | 3244737-2.patch | 400 bytes | andypost |
Comments
Comment #2
andypostFix line-endings in patch
Comment #3
andypostneeds some test to extend
Comment #4
droplet CreditAttribution: droplet commentedComment #6
Kristen PolThanks for the issue and patch. Tagging for testing, updating the issue summary to use the template, and add screenshots.
Seems like it would be hard to add a manual test but I'll leave that to someone who writes tests to decide :)
Comment #7
longwaveConfirmed the behaviour manually, and added a functional JS test.
What should happen is that disabled pre-selected checkboxes appear for other roles when you select the "authenticated user" role for any permission, as that overrides all other roles. Without the patches the dummy checkboxes don't appear; instead the per-role checkboxes just disappear, which isn't as clear to the user as to what it means.
Comment #9
Satyajit1990 CreditAttribution: Satyajit1990 at Material for Salsa Digital commentedHi posting my QA observation before the patch
Testing steps before patch :
1. Go to the admin site.
2. Go to admin/appearance >> Enable the Seven theme as default
3. Go to the people >> Permission tab
4. Now check the checkbox for the Authenticated user and observe for other checkbox permission
5. Now issue is after checking the Authenticated user other checkbox are getting disappear.
Testing Result after applying patch :
1. After applying the patch and after checking the AUTHENTICATED USER checkbox now other checkbox is visible and by default CONTENT EDITOR checkbox also getting checked and disabled.
2. Secondly there is no load response upon check and uncheck on network tab.
Comment #10
Kristen PolThanks for testing @Satyajit1990, but you don't need to check any boxes.
The issue is that the file
misc/checkbox.js
isn't loaded when you are on this page, which you should be able to see in theNetwork
tab. After the patch, you should be able to see themisc/checkbox.js
file loaded on this page in theNetwork
tab.Comment #11
joshua1234511Tested the patch with the following steps.
- Fresh Drupal install with admin theme set as seven.
- Check the network tab on permission page, misc/checkbox.js won't be loaded
- Apply the patch, clear cache and check the net work tab again.
- misc/checkbox.js should be loaded.
Note: -You will need to disable Aggregate and clear cache to see file list.
Before Patch
After Patch
Updated the issue summary with test steps.
Comment #13
Kristen PolLooks like an unrelated failure so retesting.
Comment #14
longwaveBack to RTBC, test failures were unrelated.
Comment #16
Kristen PolUnrelated failure. Back to RTBC.
Comment #18
longwaveAnd again :(
Comment #20
yogeshmpawarLooks like test failures are unrelated.
Comment #22
immaculatexavier CreditAttribution: immaculatexavier as a volunteer and at Material for Drupal India Association commentedFixed the below issue
Drupal\Tests\quickedit\FunctionalJavascript\QuickEditFileTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-147.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.19 �[44m#StandWith�[0m�[43mUkraine�[0m
Warning: Your XML configuration validates against a deprecated schema.
Suggestion: Migrate your XML configuration using "--migrate-configuration"!
Attaching the updated patch
Comment #23
longwave#22 is out of scope, phpunit.xml needs to remain compatible with both PHPUnit 8 and 9 for now.
Comment #26
smustgrave CreditAttribution: smustgrave at Mobomo commentedPatch #7 appears to still work on 10.1.x and was marked RTBC before #22 so putting back and hiding patch #22
Comment #27
klonosCleaned up the issue summary, and removed explicit reference to the Seven theme, as this happens with Claro as well (tested before/after patch and confirmed).
Checking the network tab in the browser for presence of the misc/checkbox.js file is one thing to test for, but testing for the actual checkbox behavior being restored is also a valid, additional thing to check and confirm.
Comment #28
klonosComment #29
larowlanI was going to ask that we also add another assertTrue for the real checkbox (i.e that it remains checked) but then read the code of user.permissions.js and saw that it doesn't do anything to the checked state, only the style properties.
Updating issue credits
Comment #33
larowlanCommitted to 10.1.x and backported to 10.0.x and 9.5.x
Comment #34
larowlan