@glena

Some comments and suggestions

1. The latest patch for this issue has some changes in AccountForm.php around

   +    if (!$user->hasPermission('administer permissions')) {
   +      $form['account']['roles']['administrator'] = [
   

   but the administrator role is not necessarily 'administrator', it is configurable per site (see /admin/people/role-settings )

   Therefore, whilst !$user->hasPermission('administer permissions') checks if the user doesn't have the site's administrator role, the next line doesn't necessarily call on this same role.

   The best I've come up with to fix this is

   +    if (!$user->hasPermission('administer permissions')) {
   +      $roles = \Drupal\user\Entity\Role::loadMultiple();
   +      foreach ($roles as $role) {
   +        if($role->isAdmin()){
   +          $current_admin_role = $role->id();
   +        }
   +      };
   +      $form['account']['roles'][$current_admin_role] = [
   

   but I've resisted making and submitting a patch with this change, as there must be a more succinct (one-liner) to determine the site's administrators role id. Suggestions?
   
   

2. In reference to the concern raised in #68 point 2 by @catch, I support an upgrade path too (is that an update hook?) that assigns this new permission (‘administer user roles’) to anyone who currently has the ‘administer permissions’ permission.
   
   

3. Do we want to reference #2846365 in the patch when we don't know what (or even when) a finished solution could come from that separate issue? @catch in #68 point 3 notes how 🐛 [regression] User roles field access is inconsistent, users with 'administer users' permission can gain full access Needs work doesn’t even touch the AccountForm.php.

   Wouldn't we want to keep references for the other issue out of this issue's patch?
   
   

Whilst we use notifications for state change transitions, we also use notifications specifically for same state transitions. This isn't a bug for us, it's currently logically working as per the granular one to one transitions we have setup (and have associated to notifications), and it seems this change would break this current feature for us. For context: these notification emails (for same state to same state) have email body content derived from other entities (through various custom twig functions) associated to this page. Yes, we could add this information onto the authenticated user’s page render, but this doesn't notify all reviewers with this new information (with notifications what this module is all about). We therefore have users editing content (e.g. transition state first_reviewer to state first_reviewer) that still need a notification.

Even though users could configure multiple transitions (to avoid same to same conditions) (like @drclaw #7 noted earlier) and associate applicable ones to notifications there seems to be appetite for this change. Therefore, as part of this fix can there also be an option to continue to allow same to same transitions. e.g. additional code changes for this issue to add a checkbox option with a label of "Send notification even if transition is to the same state" for each configured notification. In this way those of us who want the transitions to same states to still occur (the number that require this is somewhat unknown as I doubt module users would actively be looking for an issue like this) can have an option to continue to allow them.

Unfortunately, at this stage, understanding many elements of this bug are beyond my capabilities, but I can confirm #3277784 caused a number of sections, on a site I manage, fail to render after upgrading to 9.5.9. Issue summary above seems to align with the problem I've encountered. Applying the reversion patch in #2 fixes all issues and I'll be using that so I can take the site to 9.5.9 for now.

Using patch #5 the missing sections of the site are rendering again, but the patch creates new problems with the site breadcrumbs. The site has both current_page_crumb and menu_breadcrumb installed and with patch #5 applied (no issues with #2 reversion applied) I'm getting duplication on the last breadcrumb entry and only on term pages. Sorry, I have not been able to isolate why.

Given upgrading from 9.5.8 (or prior) to 9.5.9 can result in parts of a page no longer rendering (causing a blocker to upgrade for some) is Normal Priority flag sufficient for this issue?