🇩🇪Germany @gngn

I agree with @jsimonis: it would be nice to have an new version (2.0.0-beta1 is almost a year old).

Patch as described above.

gngn → created an issue.

I created a MR.
hope everything went OK (I'm still not used to MRs).

I reopen this for 3.0.x because it is still an issue, I do not think the other modules are going to provide access checks and I really like to have a clean security review ;)

The patch is a bit smaller than the one provided for 8.x in #2 (mainly because we already have config for the views access check).
Like #2 you can enter whole views or distinct displays to ignore (i.e. "webform_submissions" or "webform_submissions:embed_default").

I also updated the description.

Sorry, but I don't quite understand.
Is there a replacement for algolia/places, i.e. can I use webform_location_places without using algolia/places?
I'm not sure but briefly looking at the code it seems webform_location_places requires algolia/places.

I'd like to keep using webform/composer.libraries.json (to easily get library update when updating webform) - so maybe we should offer a patch to webform/composer.libraries.json?
Also a clean status page gives me a warm feeling.

Or am I getting something wrong?

Thanx for any explanations.

Just because I stumbled upon it: the link in #2 is truncated.
I think it should be
/docs/drupal-apis/services-and-dependency-injection/altering-existing-services-providing-dynamic-services →

gngn → created an issue.

The commited patch did not work for me.
I got the error:

Uncaught PHP Exception TypeError: "PhpOffice\\PhpSpreadsheet\\Worksheet\\Worksheet::setCellValue():
  Argument #3 ($binder) must be of type ?PhpOffice\\PhpSpreadsheet\\Cell\\IValueBinder, string given,
  called in modules/contrib/xls_serialization/src/Encoder/Xls.php on line 171"
  at /vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Worksheet/Worksheet.php line 1176

That is because Worksheet::setCellValue($coordinate, $value, ?IValueBinder $binder = null) expects the first parameter $coordinate to be:
array|CellAddress|string $coordinate Coordinate of the cell as a string, eg: 'C5'; or as an array of [$columnIndex, $row] (e.g. [3, 5]), or a CellAddress object.

So we need to change:

-        $sheet->setCellValue($column, $i + 2, $this->formatValue($value));
+        $sheet->setCellValue([$column, $i + 2], $this->formatValue($value));

This is done in 🐛 Tests are failing on 10.1 Fixed . So you need to also apply the patch from this issue (after the patch from here).

MR 27 worked for me, thank you.

Added tag Drupal 10 compatibility.

Patch like proposed above.

gngn → created an issue.

#28 worked for me (with potx 8.x-1.0).

I just noticed that 📌 Support Drupal 9 Fixed also changed src/Form/ChangeAuthorActionForm.php besides src/ChangeAuthorAction.php.

So I am only proposing to revert parts of #3197015 (changes in ChangeAuthorActionForm.php should be kept).

Sorry about that, I am also changing issue subject and description.

Patch as proposed.

I opened 🐛 $this called in non object context - revert #3197015 and add missing translation Active .

gngn → created an issue.

With #2 we are using $this->t() and $this->messenger in a static function, i.e. we are not in object context.

I also think we do not need the newly introduced dependency injections at all:

• MessengerInterface $messenger is only used in public static function changeAuthorActionFinishedCallback(), so not in object context.
• TranslationInterface $stringTranslation is not used at all (because #2 also has "use StringTranslationTrait" which would be enough for the call to $this->t() in the same function ...
• ... but this also happens in the static function, so we cannot reference $this at all

There is still one untranslated string 'Changing author...' in public static function updateFields().

This considered I think we should revert this commit (and $message = 'Changing author...'; to $message = t('Changing author...');.

Or did I get something wrong?

Patch coming.

I encountered similar behaviour: LDAP is not updated after changing the password.

I logged out and logged in again (only possible with the new password).
After that LDAP was synced.

So I think the password change was only stored in the drupal database at first.
The re-login somehow triggered the LDAP sync.

I just noticed that the patches in 🐛 Unable to reset user password on non-ldap accounts Postponed: needs info also include this change (comments #11 and #12).

#3251947 also includes one other change.

I just noticed that the second part of the patch (added by ilya.no in #11) is the same as in 🐛 Unable to change admin user password RTBC (which is RTBC).

#11 worked for me.

Attached re-roll against current dev.

@droath Thank you for commiting the patch from #3344350.
That solution looks cleaner - but it lacks the change in the annotation:

    *
-   * @return array|null
+   * @return array
    *   An renderable array of the view.
    *

Also it would have been nice to be mentioned in the commit ;)

+1

#11 worked for me too.

In my case the revision user had been deleted (so getDisplayName() was called on null).

Like @tekNorah wrote in #6 the trait BundlePermissionHandlerTrait class does exist for Drupal 9.
It was introduced in 9.3.0 (so I am adjusting the issue title).

Current 4.0.14 announces compatibility with 9.2 (issue #3301118: Drupal 10 compatibility → ):
core_version_requirement: ^9.2 || ^10.

So I think we should raise that to 9.3
core_version_requirement: ^9.3 || ^10.

Btw: Using above posted repo searches show:

Advanced search is disabled since 9.5.x is not the default branch

I think that's the reason for not finding "trait BundlePermissionHandlerTrait".

In #3 johnv states:

It is possible that endhours or starthours are empty (value NULL), in that case the sorting is not executd, due to the isset()

If endhours or starthours are empty (value NULL) we cannot compare the values - so we cannot sort by 'em.
Patch #2 than falls back to the behaviour without the patch - i.e. same day means "equal" (return 0).

We could implement something like not null value is greater than NULL or something similar.

Or what am I not getting?

Patch like proposed above.

I did not test with exceptional day or seasons.

gngn → created an issue.

Patch as proposed above.

gngn → created an issue.

I think this should also mention the info stated in #3012631: smtp_on option still sends mail on 8.x-1.0-beta4 → :
to disable SMTP via settings.local it is not enough to overwrite 'smtp.settings smtp_on'

$config['smtp.settings']['smtp_on'] = FALSE;

but you also need to overwrite 'system.mail interface.default':

$config['system.mail']['interface']['default'] = 'php_mail';

Thank you for the quick response.

I know "Minimally maintained" ... but there are still a lot of installs and I would like to have the status message...

#3 modifies swiftmailer_requirements($phase) to

    if ($phase == 'install') {
      ...
    }
    elseif ($phase === 'runtime') {
      ...
+  else {
+    try {
+      ...
+    }
+  }

(at least with current 8.x-2.x).

According to API: hook_requirements $phase can have the following values:

• Checking installation requirements ($phase == 'install').
• Checking update requirements ($phase == 'update').
• Status reporting ($phase == 'runtime').

The above codes will only run in phase 'update'.I think we need the 'runtime' phase, so we should get rid of the 'else' - or what I am not getting?

P.S. Changing to current 8.x-2.x

I've been using #9 since december 2021 and #31 since november 2022 without problems.

So I agree with pahles: please commit and release a new version.

Update: in my case the user I tried to masquerade as has been blocked in the meantime.

So if we are trying to masquerade as a blocked user, we still get the message "You are masquerading as..." but cannot log in.
Worse we are also logged out of the masquering account (probably an admin).

This has been addressed in 🐛 Error out when trying to masquerading as blocked user Needs review but the patches there are way too old to apply.
I submitted a patch in that issue comment #15.

New patch applying to current 7.x-1.x-dev and 7.x-1.0-rc7

Includes:

• Require users.status = 1 in masquerade_autocomplete() and masquerade_autocomplete_multiple(), so you do not get any blocked users via autocomplete.
• Validate user's status in masquerade_block_1_validate(), so you will get a form validation error if you entered a blocked user.
• Check user's status in masquerade_switch_user().

I do not think that we need to log to watchdog (above patches did).

I have one question: the other patches checked user's status like:

if (!empty($new_user->uid) && empty($new_user->status)) { ... }

What's the purpose of !empty($new_user->uid) &&?
My patch just checks empty($new_user->status).

Same here.
Masquerade (7.x-1.0-rc7 ) used to work for a long time, but now the masquerading user is logged out immediately.

Issue 🐛 Fix check on the user permission for vocabulary on select switcher vocabulary Fixed proposes the exact same patch as above.
Since that one is already Reviewed & tested I am closing this issue here.

Updated description.

I can also confirm that #2 solves the problem.

I proposed an identical patch in 🐛 Permissions per vocabulary in toolbar's select Closed: duplicate (I am closing that one since this issue here is already Reviewed & tested).

I think the handling of 'scanning published contents' in LinkExtractorBatch::processEntities() starting in #17 assumes that the scanned entity is a node - but it could be any entity type (e.g. paragraph).

I tried to illustrate the problem with "COMMENT":

    $entityTypes = $this->getEntityTypesToProcess();
    [...]
    // COMMENT: naming this $publishedNodesOnly - setting name use "content"
    $publishedNodesOnly = $this->linkcheckerSetting->get('search_published_contents_only');

    // COMMENT:looping through different entity types
    foreach ($entityTypes as $entityTypeData) {
      [...]
      // COMMENT: storage for current entity type
      $storage = $this->entityTypeManager->getStorage($entityType->id());
      foreach ($ids as $id) {
        // COMMENT: $id is ID of current entity type (and we're loading it)
        $entity = $storage->load($id);
        if ($entity instanceof FieldableEntityInterface) {
          // COMMENT: $entity is a FieldableEntity
          if ($publishedNodesOnly) {
            // COMMENT: $entity->id() is the same as $id (since we loaded $entity with $id) ...
            $nid = $entity->id();
            // COMMENT: ... but now we call it node-ID - and try to load this node!
            // That is only correct if current entity type really is node - but it could be anything.
            $node = $this->entityTypeManager->getStorage('node')->load($nid);
            [...]
          }
        else {
            // Handle $entity like before this patch
            [..]
          }
        }
        [..]
      }
      [..]
    }

Or am I getting something wrong?

Trying to sort out the diffs between #19, #20, #21, #22

• @JurgenR wrote in #20
  

  Patch #19 can't apply on 8.x-1.0-beta4.
  This is caused by changed line numbers, I've updated the patch to work against 8.x-1.0-beta4.

  But #20 differs from #19 by missing the following change in #19 in js/menu_link_weight.js:

           // Checking and unchecking the checkbox may also update the link title.
           $checkbox.on('change', function() {
             if ($checkbox.is(':checked') && !$link_title.data('menuLinkAutomaticTitleOverridden')) {
               if ($title.val() !== undefined) {
  -              $current_selection.html($title.val().substring(0, 30));
  +              $current_selection.html(Drupal.menu_truncate($title.val(), $truncate, $add_ellipsis));
               }
             }
           });

  So #20 drops the support for "Checking and unchecking the checkbox" (by keeping the hardcoded 30).

• 8.x-1.0-beta5 added a check $title.val() !== undefined) to the same part:
  

  <             $current_selection.html($title.val().substring(0, 30));
  ---
  >             if ($title.val() !== undefined) {
  >               $current_selection.html($title.val().substring(0, 30));
  >             }

  This change made it into #21 (meant for 8.x-1.0-beta4):

           // Checking and unchecking the checkbox may also update the link title.
           $checkbox.on('change', function() {
             if ($checkbox.is(':checked') && !$link_title.data('menuLinkAutomaticTitleOverridden')) {
  -            $current_selection.html($title.val().substring(0, 30));
  +            if ($title.val() !== undefined) {
  +              $current_selection.html($title.val().substring(0, 30));
  +            }
             }
           });
         });

  which still keeps the hardcoded 30.

  #21 also changes the behavoiur of Drupal.menu_truncate - I think that is meant by
  

  ellipsis behaviour was not working as expected when dealing with long words

  • #19 and #20 had:
    

    +  Drupal.menu_truncate = function (title, max_length, add_ellipsis) {
    +    var truncated = jQuery.trim(title).substring(0, max_length);
    +    if (add_ellipsis) {
    +      // Thanx to
    +      // https://stackoverflow.com/questions/2248742/jquery-text-truncation-read-more-style/2249591#2249591
    +      truncated = truncated.split(" ") // separate characters into an array of words
    +        .slice(0, -1)  // remove the last full or partial wordc
    +        .join(" ") + "..."; // remove the last full or partial word
    +    }
    +    return truncated;
    +  }

  • #21 has:
    

    +  Drupal.menu_truncate = function (title, max_length, add_ellipsis) {
    +    var ellipsis = add_ellipsis ? "..." : "";
    +    return jQuery.trim(title).length > max_length ? title.slice(0, max_length) + ellipsis : title;
    +  };

  I did not test this change.

• #22 is #21 applied to 8.x-1.0-beta5 (I think).
  Since 8.x-1.0-beta5 added the mentioned check $title.val() !== undefined) #22 does not include this change.

So I think we should

1. Reintroduce the mentioned use of Drupal.menu_truncate() (introduced in #19, dropped in #20, #21, #22).
2. Decide wether ellipsis behaviour is better handled in #19 or in #21.
   @JurgenR can you name an example for ?

Patch coming.

I added a version of #75 for current 7.x-1.x-dev (26 February 2010).

No changes to #75 (i.e. just the diff line numbers).

I am using #19 with 7.x-2.0-beta8 and can confirm that it finds added files.