🇫🇮Finland @hartsak

hartsak → created an issue.

I'm in the same boat with dabodia and andyf about the default setting value, but of course I can also understand other opinions about it.

Anyway, if my options are to use a patch or go and change some setting value, I'd much rather change that setting.
I have now 2 projects with this issue and I'm using a patch, so I'd be happy to get rid of that patch :)

I haven't had any issues with the patch from #6, so I'm not against committing it to the module!
If it was officially committed to the module I wouldn't have to be using the patch.

I tested the new setting in my project and it seems to work - so thanks a lot!

When it's off by default at least it shouldn't break anyone's theme when they update from older version (1.x) to new version (2.x).
However, I'm not sure if there would be any better wording for the setting title, but as a non-native English speaker I wasn't able to come up with better suggestions :)
The way it is now is probably good enough, I think!
-> "Try to prevent text wrapping separating the last word from the icon."

I encountered this same issue in my project after I updated extlink module from 1.x to 2.x.
I'll try to add a screenshot where the issue can be seen. Basically a span with a class "extlink-nobreak" wraps the last word of the link and white-space disappears merging the words in the link together.

It seems some side effects were already anticipated in the issue https://www.drupal.org/project/extlink/issues/3172378#comment-15757800 📌 Prevent extlink icons from breaking into new lines Needs work

While the provided patch here "solves" the issue by removing the wrapping span, I guess a more thorough solution might still be better? Like adding another checkbox to the module where it could be selected if that span should be added there or not (default: false)?

hartsak → created an issue.

hartsak → created an issue.

Ok, here's another try. I tried to combine your patch with mine. Hopefully I didn't mess up something there...

And by the way, how to use the latest version in tests? Now it seems only "media_entity_dreambroker 8.x-1.x-dev" is available in the "Test with" drop-down?

Oh, you were fast! Nice that you have taken into account the translations too, I completely forgot about them.
I can test your approach too and provide a new patch.

Here's a simple patch for starters. Trying to use the media entity label for the title and providing a default value "Video" (translatable).
Also, noticed some problems in tests (unrelated) and tried to fix them too.

hartsak → created an issue.

Thank you very much for the reply @drunken monkey!

It's just as you said - one of the fields enabled in the highlight processor was indeed a "Search api attachments" field. I hadn't paid any attention to those settings as I wasn't the original creator of that particular search index. That might very well be the reason why the highlight filter caused some performance issues in my case. I did a double check of that (it's always good to conduct testing in a production environment...) by enabling the highlight filter again and looking at the transaction data in New Relic.

After I selected these in the "Exclude fields from excerpt" options the performance issues were gone:
* rendered html output (might have very well contained some attachments in some cases)
* search api attachments fields (important!)
* taxonomy reference fields (not completely sure was this needed too, but it might have helped)

And just for the record I have the following module versions enabled:
* search_api: 8.x-1.31
* search_api_solr: 4.3.1
* search_api_attachments: 9.0.2
* Drupal: 10.1.6
* SOLR: 8.5.2

So I guess this ticket is no longer needed, as the problem is solved in my case. Thanks a lot for help!

In case this helps a bit more, here are all the items from the same slow stack. I realized I had previously only added one of these as an example but I don't know how to get this information out of New Relic in a better way.

Could it be that I have too many (fulltext) fields where the highlight processor should run?

1. Drupal\search_api\Query\Query::postExecute
2. Drupal\search_api\Entity\Index::postprocessSearchResults
3. Drupal\search_api\Plugin\search_api\processor\Highlight::postprocessSearchResults
4. Drupal\search_api\Plugin\search_api\processor\Highlight::addExcerpts
5. Drupal\search_api\Plugin\search_api\processor\Highlight::getFulltextFields
6. Drupal\search_api\Utility\FieldsHelper::extractItemValues

hartsak → created an issue.

Just adding some findings here after a quick debugging. Maybe these will help in solving the issue?
In my case, it looks like the extra setting for "title" has been saved as a boolean value where the code assumes it is a string.

For example from the config yml file of my "viewsreference" field:

enabled_settings:
    argument: argument
    title: true

In this example the argument works as expected, but the title doesn't.

Because of that, in the node edit form, the "Include view title" field doesn't appear. The setting for the title doesn't get through in line 154 of file
viewsreference/src/Plugin/Field/FieldWidget/ViewsReferenceTrait.php

foreach ($enabled_settings as $enabled_setting) {
  if (!empty($plugin_definitions[$enabled_setting])) {

In addition to that, the setting for title doesn't work when it is passed forward with the $enabled_settings in the file
viewsreference/src/Plugin/Field/FieldFormatter/ViewsReferenceFieldFormatter.php

$view->element['#viewsreference'] = [
  'data' => unserialize($item->getValue()['data'], ['allowed_classes' => FALSE]),
  'enabled_settings' => $enabled_settings,
  'parent_entity_type' => $parent_entity_type,
  'parent_entity_id' => $parent_entity_id,
  'parent_field_name' => $parent_field_name,
];

Not sure yet how this could be fixed, as simply manually fixing the config file and doing a config import doesn't seem to help. I have added a quick hotfix that helps in my case, but this only helps with the symptoms not with the cause. So if others have the same issue too, you can try the hotfix but a proper fix would be better :)

Yeah, I have exactly the same problem as #6.

Nice! It seems that the errors are gone when I use the patch from #12.
I tried with openid_connect_windows_aad 2.0.0-beta6 and without the "quick fix" from #4.

Thanks for the fix!

Hello @kekkis!
I finally had time and an opportunity to test the issue fork in my D10 (10.1.5) project. Works as expected: D10 compatible, both quality and lang parameters work. The videos also work without the additional parameters.

Thank you for your efforts in maintaining this!

Yes, I think that the "Manual mappings" textarea from openid_connect_windows_aad module works better for the long hashes used in Azure groups. Having those two keys sounds like a good idea if it allows us to continue using the textarea in the AAD settings for role mappings.

Unfortunately my project is currently in a state where I can't really do any debugging until I get the D10 update to progress further. I think I can still use it for testing some patches etc. when they are available if it is of any help!

Sorry about mixing things up a bit, but I just noticed that the patch in this issue also fixes a lot of Drupal 10 compatibility issues. Because this project doesn't have the Automated D10 patch, I guess we could use this issue to come up with a simple D10 support at the same time?

I checked the patch from #5 with Upgrade Status module and only thing missing from it is the version number. So here is another patch that should add it as well. After that this could be used for Drupal 10 upgrade too?

> -core_version_requirement: ^8.8 || ^9
> +core_version_requirement: ^8.8 || ^9 || ^10

Thanks for the new release @webflo!

I also tried the latest openid_connect_windows_aad 2.0.0-beta6 (with openid_connect 3.0.0-alpha2) and the same PHP error still persists when logging in as an Azure user.
Warning: Array to string conversion
in
Drupal\openid_connect\OpenIDConnect->saveUserinfo()
here -> openid_connect/src/OpenIDConnect.php

If I use the quick fix from #4 I get a new error with the openid_connect_windows_aad 2.0.0-beta6:
TypeError: Cannot access offset of type string on string in function openid_connect_windows_aad_openid_connect_userinfo_save() (line 70

At this point it looks like I can only get the logging in part to work without errors if I use openid_connect_windows_aad 2.0.0-beta5 and the quick fix from #4.

Maybe there's still something wrong with the role mapping (or something else) at least in my end. I have set the role mapping in the general (experimental) OpenID connect settings and I tried with and without them in OpenID AAD settings. My project is still D9 (I'm in the process of updating it to D10).

Thanks @mrinalini9! The patch in #5 seems to work.

Tested the fix in a clean project and it works. Thanks @scott_euser for the fix!

The quick solution in #4 helped in my case, thanks @bbu23 very good instructions!

Thank you for the quick reply @WalkingDexter !

I'm probably going to re-think if I really need the message feature in the future. In the meantime, I will continue using the 1.x version and see if I'm able to patch it with the D10 patch from here:
https://www.drupal.org/project/content_translation_redirect/issues/3369274 📌 Automated Drupal 10 compatibility fixes Closed: won't fix

Great work with the module so far and have a nice weekend!

hartsak → created an issue.

Ok, another try.
The first patch was a failure as it would not allow using DreamBroker videos without lang and quality parameters. That should be fixed now. In addition, I tried to fix deprecated/broken tests. So let's see how this attempt turns out (fingers crossed).

Here is a quick fix that allows using just those mentioned parameters - "lang" and "quality".
Obviously, this could be made much better, but like mentioned, this is a quick fix :)

hartsak → created an issue.

Thanks for the fix! I just encountered this issue in my project and the fix in #24 solved my problem.