If someone needs a patched 1.6.0.
The problem described here causes an error in Drupal 11. I think that's enough to bump the priority.
TypeError: Drupal\Component\Utility\Html::escape(): Argument #1 ($text) must be of type string, null given, called in /var/www/core/lib/Drupal/Component/Render/FormattableMarkup.php on line 238 in Drupal\Component\Utility\Html::escape() (line 431 of /var/www/core/lib/Drupal/Component/Utility/Html.php).
Rerolling the patch for 11.1.
walkingdexter → created an issue.
The previous patch doesn't work with Composer due to version string. This one should work.
Rerolling the patch for 2.0.0
@iseeaflyingcrane Thanks for the issue! This is a valid point.
The reason for this behavior is that product variation does not have a canonical link template. However, if an entity doesn't have a canonical template, it can still have a canonical link. EntityHelper::supports()
excludes entities that can be successfully added to the sitemap.
@gbyte Maybe we should support all content entity types? We can move entity types without a canonical link template to a separate table on the /admin/config/search/simplesitemap/entities page and warn users about possible errors.
@gbyte Yes, it makes sense.
The desired result can be achieved by using hook_entity_query_tag__TAG_alter
or hook_entity_query_tag__ENTITY_TYPE__TAG_alter
, where TAG
is simple_sitemap
. For example, for nodes the hook will be hook_entity_query_tag__node__simple_sitemap_alter
. In this hook you can add a date condition to the query.
The above feature was added in ✨ Allow to alter entity url generator query Needs review and will be available in the next release.
See related issues for details.
Already fixed in ✨ Add Senzam search engine to IndexNow functionality Fixed . Marked this also as fixed to save credits.
Thanks for the clarification. This seems to be a very specific use case. If I understand correctly, the problem can be solved with a few lines of code using hook_simple_sitemap_links_alter()
. Is this true?
I can't reproduce the problem by following these steps with Menu Item Extras 3.1.0. Please provide more information.
I believe this issue 🐛 option argument 'bundle' sometimes has a NULL value, which can cause issue Fixed covers a similar state, and they proposed to solve it by falling back to the entity type ID.
In this case the problem is more obvious, as the documentation says that FieldDefinitionInterface::getTargetBundle()
can return NULL.
@WalkingDexter @dpi please check out the gitlab review. Let's not merge this before discussing if this should instead go into a 3rd party simple_sitemap_monitoring module which I feel should be the way to go ATM.
I think this functionality can be added directly to the Monitoring module. It already contains plugins for other contrib modules.
Merged, thanks!
This is not a breaking change because PHP is case insensitive for the class names. However, in some environments, errors may occur due to file name changes. See 💬 Class 'Drupal\simple_sitemap\Queue\SimpleSitemapQueue' not found Fixed for additional information. This should be described in the release notes.
Need more information about the use case. Why can't this be solved with the "Excluded languages" setting?
The proposed solution should not replace the current routing. It should be an option.
I don't see any practical use for this feature. This array is not intended to be modified. Feel free to reopen if you can provide a use case.
Need more information about the use case and the steps to reproduce.
The array of sitemap links should not be modified inside the addChunk()
method.
8.x-3.x is outdated, it's unproductive to spend time on it. MR needs to be reworked for 4.x.
The necessary hooks are already in the core. All we need to do is add tag and metadata to the query.
walkingdexter → created an issue.
Fixed, thanks!
I can't reproduce the problem on a clean install. Please provide steps to reproduce.
Committed, thanks!
ProcessOutbound Case:
Create a view with the URL of foo/bar/sitemap.xml. Try to go to this URL and you will be redirected to /bar/sitemap.xml.
FYI, that's not how it works. Outbound processors cannot be the cause of a redirect.
Need steps to reproduce. The queue is cleared before rebuilding.
Feel free to reopen if you have a problem with multisite installations.
The current solution is correct. The problem must be fixed by a third party #3228568: LanguageNegotiationCountryPathUrl plugin not returned on Language Negotiator getNegotiationMethods() call → .
Not related to 4.x
, as sitemap types are not plugins.
walkingdexter → created an issue.
walkingdexter → created an issue.
The sitemap protocol still contains <priority>
and <changefreq>
values.
Closing due to lack of activity.
Closing due to lack of activity.
This feature conflicts with ✨ Override a non-indexed bundle on a per-entity basis Needs work and therefore cannot be implemented.
walkingdexter → created an issue.
Fixed errors and merged, thanks! We can't use autoconfigure
for now because it would break backwards compatibility.
walkingdexter → made their first commit to this issue’s fork.
Merged, thanks!
walkingdexter → made their first commit to this issue’s fork.
Just realized that this is a duplicate of ✨ change sitemap.xml?page=1 to sitemap1.xml Needs review .
My current thoughts:
- In this issue we should focus on adding the ability to act on a processed entity. I propose to move the event subscriber (or hook implementation) to the Rabbit Hole module by creating a separate issue. Otherwise, I don't think it's right to drop support for Rabbit Hole 1. It's stable, and we'll have to support both versions.
- The discussion about events and hooks is relevant again because of this change record → .
- The proposed code must use all possible type declarations.
- All warnings from the CI pipeline must be fixed.
When a developer decides to use DATE_ISO8601
, he will see that DATE_ISO8601
is deprecated and DATE_ATOM
should be used instead. However, the <lastmod>
date can be formatted in different ways and ATOM format is not the only option.
@introfini Sorry for the late response. All you need to do is add a custom submit callback to the node form and change the $form_state
values related to the sitemap settings before they are saved.
There is no need to divide sitemaps by language. You can simply divide them by content type. URLs for other languages will be automatically included.
The sitemap page itself is not supposed to be indexed. See related issues for details.
The sitemap page itself is not supposed to be indexed. See related issues for details.
This looks like a feature request.
@damienmo The following may help you:
- Set the "Maximum links in a sitemap" limit or reduce it if it's already set.
- Turn off the "Exclude duplicate links" feature. With large numbers of links, this feature will lead to heavy SQL queries and memory issues.
If that doesn't help, try to find out what the largest data is.
Merged, thanks!
@handkerchief In your scenario I still get the 404 error. Can you provide more details on step 3?
- Is the Content Translation module disabled?
- What settings are set on /admin/config/regional/content-language?
- What settings are set on /admin/config/regional/language/detection?
I tested the following scenario on a clean install with 4.x-dev
:
- Enable two languages - English and non-english.
- Set English as default language.
- Use the "Selected language" plugin for language detection and configure it with a non-english language.
- Enable content translation for the "Article" content type.
- Add a new node of type "Article" and translate it into a non-english language.
Now when I set a URL alias for the created node, I get a 404 error if the alias has a non-english language. If I change the alias language to English or unspecified, then the 404 error does not occur. The sitemap is also correct in this case.
So I can't reproduce the problem on a clean install. Maybe I'm missing something. Feel free to reopen if you can provide more information to reproduce the problem. In other cases see #4 and #5 for possible solutions. Also consider changing the URL alias language.
Merged, thanks!
I can't reproduce the problem from #4 - different sitemap results for different languages. This has probably been fixed in other issues. Speaking about the original problem, the proposed resolution is incorrect. If I understood the example correctly, then https://www.drupal8.loc/de/OeffentlicheSeite
and https://www.drupal8.loc/en/node/401
are the URLs of the same node. In this case, the specified result is expected. This is exactly how the "Skip non-existent translations" functionality is designed.
Just tested, all bugs reported here have been fixed in 🐛 Unexpected language prefixes on sitemap index Fixed
@hmendes Instead of a class, you should use an interface.
walkingdexter → created an issue.
Simplified and committed, thanks! Thoughts from #21 are respected. Credits saved for the guys from ✨ New permission to only edit entity sitemap settings Needs review .
@selinav Sorry for the late response. There is currently no way to specify the date of last modification for views pages without custom code. However, I assume that this feature will be added one day. One possible approach is to get the date from entities that are displayed in a view.
@cosmicdreams This error is not related to the Simple XML sitemap because the ext-xmlwriter
requirement is explicitly specified in composer.json
. Also note that this PHP extension is enabled by default. If it was previously disabled, then it must be enabled again.
@_renify_ Please provide more information about the proposed changes. Are you looking for a way to add views pagination pages to your sitemap?
@_renify_ Please provide more information about the proposed integration.
Oops, just realized that we already have an issue for this.
@berdir Thanks for the issue!
@gbyte I think the whole code in \Drupal\simple_sitemap\Manager\EntityManager::setBundleSettings()
after $bundle_settings->save();
should be removed.
- There is no reason to delete entity overrides in case bundle indexation is disabled. User can re-enable bundle indexation and overrides will be needed again.
- There is no reason to delete entity overrides which are identical to new bundle settings. User can change bundle settings and overrides will be needed again.
We also have a potential performance problem with \Drupal\simple_sitemap\Manager\EntityManager::removeBundleSettings()
but this is a rare case. Also this shouldn't be a problem after closing
#3034070: Store entity instance settings as fields on entity →
.
Already fixed in 📌 Fix PHPStan errors Fixed , see this commit for details.
I can't reproduce the problem on a fresh install (did a quick debug for the views exposed form). According to the error trace, the problem is related to starting a new session at the end of the request. This can cause many problems and not only for this module. Feel free to reopen if you can provide additional details.
walkingdexter → created an issue.
The post_update hook was used for a reason 🐛 Update from 2.x or 3.x: Export does not get imported Fixed .
See 📌 TypeError: Argument 1 passed to Drupal\simple_sitemap\Form\Handler\EntityFormHandlerBase::isSupportedOperation() must be of the type string, null given Closed: works as designed for details. Feel free to reopen if you can provide additional information on how to reproduce the problem.
It looks like we have a problem with multidomain/multilingual installations. However, the proposed resolution doesn't seem to be the best choice in my opinion. I need a second pair of eyes here.
Closing as per lack of activity. Feel free to reopen if you can provide more details about the problem.
Closing as per lack of activity.
I'm not sure this change is necessary. A similar result can be obtained by overriding the simple_sitemap.form_helper
service in your custom module. But let's take a second opinion.
Added BC layer and committed, thanks!
Deletion of translation entries for taxonomy.
@catcat811 I can't reproduce the problem with these steps. Please provide more details.
Please review the attached patch.
@ananya.k Your patch doesn't fix anything.
Is there any loss to be had adding a sanity check here besides a milisecond or two of time?
I'm not a fan of adding unnecessary checks that go against the documentation. Incorrect implementation can lead to errors not only with this module. The above case clearly shows that the problem is related to the third party.
I don't think it's fair to say that getOperation() returning NULL is a third party problem, since getOperation does not specify return types.
@jnicola The return type is specified in PHPDoc, which is respected by PHPStan.
The proposed resolution broke the script, as @benjifisher noted. All we need here is to make the parameter required.
Merged, thanks!
At the moment I don't see any suitable native alternative to jQuery TableSorter with similar functionality, especially in terms of accessibility (ARIA). Postponed for now.
I am not a fan of increasing MINOR when adding functionality as it seems to create a fork and two versions to maintain, correct?
There is no need to maintain two versions. We just increase the minor version and move on.
In addition people stay on the previous version forever as they often have something like "^4.3" in composer. What are your thoughts?
This always allow non-breaking updates. ^4.3
means >=4.3.0 <5.0.0
.
Also in regards to the upcoming release - we bumped compatibility from D9.3 + D10 to D10.2 + D11. What would be your preferred new version number?
According to
https://www.drupal.org/project/ideas/issues/3357742#comment-15651225
🌱
Guidelines for semantic versioning and Drupal core support
Needs review
, I prefer 4.2.0
version (increase the minor version).
We are approaching the release of Drupal 11, so should we expect stable version for this module before that release?
I don't have permission to create new releases. Only @gbyte can do this.
WalkingDexter → created an issue.
FYI 🌱 Guidelines for semantic versioning and Drupal core support Needs review