From b80bfda19d6f12263750e01ca441091a5744e52a Mon Sep 17 00:00:00 2001
From: nod_ <nod_@598310.no-reply.drupal.org>
Date: Wed, 4 Sep 2024 10:58:52 +0200
Subject: [PATCH] Issue #3464530 by nod_, mabho, nicxvan, joaopauloc.dev,
cassioalmeida, catch, sun, quietone, droplet, aaronbauman, geerlingguy:
Improve performance of the user.permissions.js script running in
/admin/people/permissions
---
core/modules/user/css/user.admin.css | 7 +
.../UserPermissionsTest.php | 25 +--
core/modules/user/user.permissions.js | 147 +++++++++---------
3 files changed, 94 insertions(+), 85 deletions(-)
diff --git a/core/modules/user/css/user.admin.css b/core/modules/user/css/user.admin.css
index ad5f95a59a7b..091da0e4a0aa 100644
--- a/core/modules/user/css/user.admin.css
+++ b/core/modules/user/css/user.admin.css
@@ -20,3 +20,10 @@
padding-bottom: 0.5em;
font-size: 0.85em;
}
+
+.permissions tbody tr:has(input[type="checkbox"].js-rid-authenticated:not(:checked)) .js-fake-checkbox {
+ display: none;
+}
+.permissions tbody tr:has(input[type="checkbox"].js-rid-authenticated:checked) .js-real-checkbox {
+ display: none;
+}
diff --git a/core/modules/user/tests/src/FunctionalJavascript/UserPermissionsTest.php b/core/modules/user/tests/src/FunctionalJavascript/UserPermissionsTest.php
index 0cac61c9a69b..d467a2fbabfa 100644
--- a/core/modules/user/tests/src/FunctionalJavascript/UserPermissionsTest.php
+++ b/core/modules/user/tests/src/FunctionalJavascript/UserPermissionsTest.php
@@ -50,7 +50,7 @@ protected function setUp(): void {
}
/**
- * Tests the dummy checkboxes added to the permissions page.
+ * Tests the fake checkboxes added to the permissions page.
*/
public function testPermissionCheckboxes(): void {
$this->drupalLogin($this->adminUser);
@@ -58,24 +58,25 @@ public function testPermissionCheckboxes(): void {
$page = $this->getSession()->getPage();
$wrapper = $page->find('css', '.form-item-' . $this->rid . '-administer-modules');
- $real_checkbox = $wrapper->find('css', '.real-checkbox');
- $dummy_checkbox = $wrapper->find('css', '.dummy-checkbox');
+ $fake_checkbox = $wrapper->find('css', '.fake-checkbox');
- // The real per-role checkbox is visible and unchecked, the dummy copy is
- // invisible.
- $this->assertTrue($real_checkbox->isVisible());
- $this->assertFalse($real_checkbox->isChecked());
- $this->assertFalse($dummy_checkbox->isVisible());
+ // The real per-role checkbox is visible and unchecked, the fake copy does
+ // not exist yet.
+ $this->assertNull($fake_checkbox);
// Enable the permission for all authenticated users.
$page->findField('authenticated[administer modules]')->click();
- // The real and dummy checkboxes switch visibility and the dummy is now both
+ // The checkboxes have been initialized.
+ $real_checkbox = $wrapper->find('css', '.real-checkbox');
+ $fake_checkbox = $wrapper->find('css', '.fake-checkbox');
+
+ // The real and fake checkboxes switch visibility and the fake is now both
// checked and disabled.
$this->assertFalse($real_checkbox->isVisible());
- $this->assertTrue($dummy_checkbox->isVisible());
- $this->assertTrue($dummy_checkbox->isChecked());
- $this->assertTrue($dummy_checkbox->hasAttribute('disabled'));
+ $this->assertTrue($fake_checkbox->isVisible());
+ $this->assertTrue($fake_checkbox->isChecked());
+ $this->assertTrue($fake_checkbox->hasAttribute('disabled'));
}
}
diff --git a/core/modules/user/user.permissions.js b/core/modules/user/user.permissions.js
index 19a56ae1dce0..e8a84607c592 100644
--- a/core/modules/user/user.permissions.js
+++ b/core/modules/user/user.permissions.js
@@ -13,80 +13,81 @@
* Attaches functionality to the permissions table.
*/
Drupal.behaviors.permissions = {
- attach(context) {
- once('permissions', 'table#permissions').forEach((table) => {
- // On a site with many roles and permissions, this behavior initially
- // has to perform thousands of DOM manipulations to inject checkboxes
- // and hide them. By detaching the table from the DOM, all operations
- // can be performed without triggering internal layout and re-rendering
- // processes in the browser.
- const $table = $(table);
- let $ancestor;
- let method;
- if ($table.prev().length) {
- $ancestor = $table.prev();
- method = 'after';
- } else {
- $ancestor = $table.parent();
- method = 'append';
- }
- $table.detach();
-
- // Create dummy checkboxes. We use dummy checkboxes instead of reusing
- // the existing checkboxes here because new checkboxes don't alter the
- // submitted form. If we'd automatically check existing checkboxes, the
- // permission table would be polluted with redundant entries. This is
- // deliberate, but desirable when we automatically check them.
- const $dummy = $(Drupal.theme('checkbox'))
- .removeClass('form-checkbox')
- .addClass('dummy-checkbox js-dummy-checkbox')
- .attr('disabled', 'disabled')
- .attr('checked', 'checked')
- .attr(
- 'title',
- Drupal.t(
- 'This permission is inherited from the authenticated user role.',
- ),
- )
- .hide();
-
- $table
- .find('input[type="checkbox"]')
- .not('.js-rid-anonymous, .js-rid-authenticated')
- .addClass('real-checkbox js-real-checkbox')
- .after($dummy);
-
- // Initialize the authenticated user checkbox.
- $table
- .find('input[type=checkbox].js-rid-authenticated')
- .on('click.permissions', this.toggle)
- // .triggerHandler() cannot be used here, as it only affects the first
- // element.
- .each(this.toggle);
-
- // Re-insert the table into the DOM.
- $ancestor[method]($table);
- });
- },
+ attach() {
+ const [table] = once('permissions', 'table#permissions');
+ if (!table) {
+ return;
+ }
+
+ // Create fake checkboxes. We use fake checkboxes instead of reusing
+ // the existing checkboxes here because new checkboxes don't alter the
+ // submitted form. If we'd automatically check existing checkboxes, the
+ // permission table would be polluted with redundant entries. This is
+ // deliberate, but desirable when we automatically check them.
+ const $fakeCheckbox = $(Drupal.theme('checkbox'))
+ .removeClass('form-checkbox')
+ .addClass('fake-checkbox js-fake-checkbox')
+ .attr({
+ disabled: 'disabled',
+ checked: 'checked',
+ title: Drupal.t(
+ 'This permission is inherited from the authenticated user role.',
+ ),
+ });
+ const $wrapper = $('<div></div>').append($fakeCheckbox);
+ const fakeCheckboxHtml = $wrapper.html();
+
+ /**
+ * Process each table row to create fake checkboxes.
+ *
+ * @param {object} object
+ * @param {HTMLElement} object.target
+ */
+ function tableRowProcessing({ target }) {
+ once('permission-checkbox', target).forEach((checkbox) => {
+ checkbox
+ .closest('tr')
+ .querySelectorAll(
+ 'input[type="checkbox"]:not(.js-rid-anonymous, .js-rid-authenticated)',
+ )
+ .forEach((check) => {
+ check.classList.add('real-checkbox', 'js-real-checkbox');
+ check.insertAdjacentHTML('beforebegin', fakeCheckboxHtml);
+ });
+ });
+ }
- /**
- * Toggles all dummy checkboxes based on the checkboxes' state.
- *
- * If the "authenticated user" checkbox is checked, the checked and disabled
- * checkboxes are shown, the real checkboxes otherwise.
- */
- toggle() {
- const authCheckbox = this;
- const $row = $(this).closest('tr');
- // jQuery performs too many layout calculations for .hide() and .show(),
- // leading to a major page rendering lag on sites with many roles and
- // permissions. Therefore, we toggle visibility directly.
- $row.find('.js-real-checkbox').each(function () {
- this.style.display = authCheckbox.checked ? 'none' : '';
- });
- $row.find('.js-dummy-checkbox').each(function () {
- this.style.display = authCheckbox.checked ? '' : 'none';
- });
+ // An IntersectionObserver object is associated with each of the table
+ // rows to activate checkboxes interactively as users scroll the page
+ // up or down. This prevents processing all checkboxes on page load.
+ const checkedCheckboxObserver = new IntersectionObserver(
+ (entries, thisObserver) => {
+ entries
+ .filter((entry) => entry.isIntersecting)
+ .forEach((entry) => {
+ tableRowProcessing(entry);
+ thisObserver.unobserve(entry.target);
+ });
+ },
+ {
+ rootMargin: '50%',
+ },
+ );
+
+ // Select rows with checked authenticated role and attach an observer
+ // to each.
+ table
+ .querySelectorAll(
+ 'tbody tr input[type="checkbox"].js-rid-authenticated:checked',
+ )
+ .forEach((checkbox) => checkedCheckboxObserver.observe(checkbox));
+
+ // Create checkboxes only when necessary on click.
+ $(table).on(
+ 'click.permissions',
+ 'input[type="checkbox"].js-rid-authenticated',
+ tableRowProcessing,
+ );
},
};
--
GitLab