Drupal 9.5 is no longer supported software. To fix this warning please upgrade to Drupal 11.
There is no patch to apply. This has been committed. Just waiting for someone to test the dev release and report that it works in Drupal 11 and we can tag a release.
Looks good. I'll give the dev release a test.
Code looks good. Needs some more manual testing still. We'll wait for Spuky to take a look.
~G
Awesome couldn't tell for certain the Diff, thanks for the extra info!
Also! Great freaking work on this merge request. Tests and everything.
My only question is on this line:
- $title = Html::decodeEntities(Xss::filter(trim($settings[0])));
Are we still escaping JavaScript if we remove that line?
greg boggs → made their first commit to this issue’s fork.
Thanks for the fix! We have 2 new issues in the current release that we want to fix before the next release. But, do let us know if y'all need a release soon to not be blocked.
greg boggs → made their first commit to this issue’s fork.
It's off topic, but I work for our library. Thanks for all the great work y'all do at City of Portland. Feel encouraged to open an MR with what you think the best fix is here.
I think it's reasonably safe to assume someone doesn't have a custom path with multiple leading slashes.
oph. Thanks for the report. We'll take a look as soon as we can. Clearly we need better test coverage for this feature.
To get security coverage, the module must have a stable release, and a maintainer of the module must have permission to opt into coverage. To get permission to opt into coverage, a maintainer has to have created at least one module that was reviewed and approved by the module review team.
If you already have permission to opt projects in, you can click a box. If not, here's the process:
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →
A small note on |raw. Raw disables twig's output escaping so users might be able to post JavaScript in the URL if you pass content through Raw. So, you'll want to make sure that content is escaped before it gets to Twig.
Saw this exception in my log slowing down my website because bots trigger this one a lot. Looks like there's been significant work done on this merge request since it's last review.
Nice.
Thanks folks!
greg boggs → created an issue.
Heck yea! Nice.
Nice. Thank you!
greg boggs → made their first commit to this issue’s fork.
Logo looks good. Needs a Merge request.
Hrm. The site structure tag is already selected.
Unless I'm just missing it, this does not appear to be in 3.3.x-dev yet? I ended up here because 3.3.x still installed varation cache and varationcache project page says:
"Please uninstall and remove this module once you are using Drupal 10.2 or higher and have no more code mentioning the Drupal\variationcache namespace."
The reason the project name was in the summary was to make the summary clear when it was displayed to users in search engines.
Here's my best attempt to comply with the request while still having a complete sentence.
"This module updates the core Breadcrumb block to include the current page title in the breadcrumbs. It comes with settings that are common features needed in crumbs."
I've also adjusted the full description to better line up with the new summary.
greg boggs → created an issue.
Adding a patch of the current MR for testing purposes
Ok, apparently I have forgotten how to patch. This one will work, I think.
mucked up the patch file last time. trying again.
The current patch makes oauth scope a required field. However, some APIs don't use Oauth scope. So this patch makes oauth scope optional. Still needs a reroll for Beta 5
The patch needs a re-roll for Beta5
Another good place to get support is in Drupal Slack in the #support channel.
https://www.drupal.org/community/contributor-guide/reference-information... →
Thanks for the great improvement!
greg boggs → made their first commit to this issue’s fork.
The patch works perfectly for us.
Translations have improved some. But, Zip Code label does not translate and there's no UI for adding a translation for this. Any suggestions on next steps to get missing Address labels translated?
Yea, the bug is that webform doesn't set a correct page title for the thank you page. I'll open a ticket for webforms for the title bugs and link them here.
When viewed in the Gin theme, the crumbs are more readable, but also strange. I think there's some bugs with Easy Breadcrumb and Webforms together
Path: https://drupal-cms-dev.ddev.site/admin/structure/webform/manage/nerd_qui...
Crumbs: Home > Administration > Structure > Webforms > Flex Your Nerdy Opinions > Flex Your Nerdy Opinions
greg boggs → created an issue.
greg boggs → created an issue.
greg boggs → created an issue.
Thanks for the review! Happy to get improvements committed if someone wants to open a merge request with fixes, even if they aren't perfect yet.
And to confirm the Glossary module is still abandoned with only a release for Drupal 7?
I only accept code from Merge Requests on my projects, but it's still helpful if someone adds code in a patch file. It could be nice if Drupal.org notified folks when they upload a patch that the project maintainer prefers a merge request with a link to documentation on how to make one.
Thanks, I'll give the patch a try.
My site doesn't throw this error on drush cron, but this looks like a good defensive code addition all the same.
This issue still exists. The code provided works to solve it. It would be ideal if this were a configuration in the UI instead of requiring custom code to do this. Since this is closed fixed by a maintainer, I won't re-open.
that would be awesome. You should contribute the work.
Node revision delete module is a popular solution and has the UI for setting rules to delete revisions of nodes which is a good starting place for this feature.
I don't see code in the current changes that assume a database. So, perhaps the comment in #18 is about surrounding code outside of these changes. I am going to mark this needs review again until it's more clear what work needs to be done on this MR.
This is a major issue. So far, none of the revision delete modules I have tested have support for Paragraphs revisions. Has anyone found a solve for this?
are you using the latest dev release or the tagged release?
Thanks for all your work on this feature. It works perfectly for us. However, the views_natural sort table is one of the largest tables in our database:
views_natural_sort ~66,998 InnoDB utf8mb4_general_ci 5.5 MiB
For context, our node revision table has 15,236 rows. So, 67k rows is quite large for our size of website. Does the module really need a table this large to function? I'm asking because the size of our database is making our website slow and I'm looking for ways to slim down our database.
After many attempts, I was able to get the uninstall through and now I have this error:
Drupal\Component\Plugin\Exception\PluginNotFoundException: The "paragraphs_library_item" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).
I cannot uninstall Paragraphs Library at all and it's creating a very large database table.
The website encountered an unexpected error. Please try again later.
TypeError: Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate(): Argument #1 ($field_definition) must be of type Drupal\Core\Field\FieldDefinitionInterface, null given, called in /var/www/html/web/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 234 in Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate() (line 546 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).
Drupal\Core\Field\Entity\BaseFieldOverride::postDelete(Object, Array) (Line: 460)
Drupal\Core\Entity\EntityStorageBase->delete(Array) (Line: 347)
Drupal\Core\Entity\EntityBase->delete() (Line: 212)
Drupal\Core\Config\ConfigManager->uninstall('module', 'paragraphs_library') (Line: 470)
That sounds perfect. Please do if you have the time and energy!
Greg Boggs → made their first commit to this issue’s fork.
Ok. I think I did that right.
For the tests, lets try again to fix what we can, for annoyingly difficult tests on old update hooks and such, lets just remove the problem tests. The important bit with the tests is that the tests pass, not that we have great coverage.
Awesome!
Greg Boggs → made their first commit to this issue’s fork.
Thanks for working on the tests Spuky. Looks like they are still failing:
1) Drupal\Tests\easy_breadcrumb\Functional\EasyBreadcrumbConfigureTest::test8006DefaultConfigurationUpdate
InvalidArgumentException: The configuration property capitalizator_ignored_words.0.0 doesn't exist.
smustgrave → credited Greg Boggs → .
Looks good, Thanks!
Greg Boggs → made their first commit to this issue’s fork.
woot woot
Greg Boggs → made their first commit to this issue’s fork.
Greg Boggs → created an issue.
Yes, that will work. Sorry I missed this question, I had not subwcribed to issues yet.
If anyone else finds this issue trying to figure out why they can't set their site hash in config, here's the answer on how to set and get your site hash now that it's in state:
https://acquia.my.site.com/s/article/360045953834-The-importance-of-uniq...
drush sget search_api_solr.site_hash
Awesome. Thanks for the update!
Looks good! Thanks for the work on this one team.
Unfortunately, they dev release is not ready yet. This issue still needs an MR and tests passing:
https://www.drupal.org/project/easy_breadcrumb/issues/3440889 🐛 Broken findMatchingRedirect() call Fixed
Here's the patch for that issue: It needs a Merge Request created still before I can add it to the dev release.
https://www.drupal.org/project/easy_breadcrumb/issues/3440889 🐛 Broken findMatchingRedirect() call Fixed
Hrm, what error did you get on the white screen?
This has been marked major for 8 years. So, maybe not major?
I fixed this is a couple of minutes by adding a filter to the author for the current interface language. I get no duplicates. I am not really sure about the joins and filters mentioned in 15, but adding this filter works for me.
looks good, can we get a MR for Git lab CI testing?
Hi Abhikeh Gupta,
Lets keep using findMatchingRedirect rather than switching to a different method while fixing this issue since findMatchingRedirect was working previously and was tested.
To improve workflow, we will need Merge Requests and not patches.
I'll get it taken care of this week.
ok got it! Sounds like we can include this as is.
Thanks!
Greg Boggs → made their first commit to this issue’s fork.
The code looks perfect! I'd merge it, but it looks like the patch file is also in the MR, and we don't want the patch file to end up in the final module.