🇫🇷France @Damien Tournoud

Account created on 27 April 2005, almost 20 years ago
#

Recent comments

🇫🇷France Damien Tournoud

Entity Reference and Views both respect access control now. Check your access control configuration.

🇫🇷France Damien Tournoud

This is going to need to be fixed in Drupal 8 first.

🇫🇷France Damien Tournoud

There is no generic solution for this "link container" problem, except by loading all the links for each user and each page (remember that the visibility of a menu link can depend both on the user and on the page, and can even depend on any other random variable outside of the control of the menu system).

We need to reduce the complexity of the problem somehow.

I suggest we only hide empty categories that satisfy the following condition: all the links below the category are:

* categories, or
* links related with a menu router whose access callback is 'user_access'

A category that doesn't satisfy those conditions will be always displayed.

With those restrictions, we can cache the visibility of the category per role. Does this seem acceptable?

🇫🇷France Damien Tournoud

Changes in #20 make sense, but this is an old patch, and its test doesn't comply at all with our (relatively new) Test Writers Guidelines.

   }
 }
 
+class AdminMenuBlockTestCase extends DrupalWebTestCase {

^^ Missing PHPDoc above the class.

+  /**
+   * Implementation of getInfo().
+   */
+  function getInfo() {

^^ We don't PHPDoc overriden functions anymore.

+    return array(
+      'name' => t('Admin menu categories'),
+      'description' => t('Confirm that administrative categories, which do not contain any visible child items, are not displayed.'),
+      'group' => t('System'),
+    );
+  }
+
+  /**
+   * Ensure that administrative menu items without visible children are hidden.
+   */
+  public function testEmptyHide() {

^^ The PHPDoc description should probably better start with test*...

testEmptyHide() is a badly choosen function name.

As an end note: it's quite fun to shoot down one of your own old patches :)

🇫🇷France Damien Tournoud

There are three bugs here:

* Drupal don't use absolute paths for loading include files, and the current path can change, especially during the execution of shutdown functions. This is solved in #259623: Broken autoloader: convert includes/requires to use absolute paths . Please try that patch.
* Access control on /admin don't work properly, probably because system_admin_menu_block_page_access() recursively calls itself. For now, I disabled that access control on /admin.
* Hide descriptions/Show descriptions toggles didn't work for anonymous users (it was relying on a $user parameter). I changed this to use session for anonymous users.

🇫🇷France Damien Tournoud

I forgot to add: "... but of course, I only want to be proven wrong!"

🇫🇷France Damien Tournoud

@boomatower: Ok for the name change, it makes sense and I don't really care :)

The objective of using system_admin_menu_block() was to limit code duplication and database queries as most as possible. Your solution was not enough, because we also need to check for access of each menu item, not just count them.

I turned that issue around over and over, and came to the conclusion there is no better way.

🇫🇷France Damien Tournoud

That issue has been identified several months ago. Here is a first patch for it.

Production build 0.71.5 2024