/**
 * Gets xmlsitemap frequency options.
 *
 * @return array
 *   Frequency options.
 *
 * @todo Document this function.
 * @todo Make these translatable
 */
function xmlsitemap_get_changefreq_options() {
  return array(
    XMLSITEMAP_FREQUENCY_ALWAYS => 'always',
    XMLSITEMAP_FREQUENCY_HOURLY => 'hourly',
    XMLSITEMAP_FREQUENCY_DAILY => 'daily',
    XMLSITEMAP_FREQUENCY_WEEKLY => 'weekly',
    XMLSITEMAP_FREQUENCY_MONTHLY => 'monthly',
    XMLSITEMAP_FREQUENCY_YEARLY => 'yearly',
  );
}
CommentFileSizeAuthor
#21 2850367-fix-changefreq-untranslated-default.patch2.12 KBDave Reid
#20 Képernyőkép_2022-09-20_09-30-31.png38.57 KBlonalore
#4 xmlsitemap_change_frequency_translatable-2850367-4.patch1.13 KBmrinalini9
#2 xmlsitemap_change_frequency_translatable-2850367-2.patch1.23 KBCyberwolf

Issue fork xmlsitemap-2850367

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Cyberwolf created an issue. See original summary.

Cyberwolf’s picture

Cyberwolf CreditAttribution: Cyberwolf at 2DotsTwice bvba commented
FileSize
xmlsitemap_change_frequency_translatable-2850367-2.patch1.23 KB
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
Status: Active » Needs review
mrinalini9’s picture

mrinalini9 CreditAttribution: mrinalini9 at Srijan | A Material+ Company for Drupal India Association commented
FileSize
xmlsitemap_change_frequency_translatable-2850367-4.patch1.13 KB

Rerolled patch #2 as it failed to apply, please review.

Status: Needs review » Needs work

The last submitted patch, 4: xmlsitemap_change_frequency_translatable-2850367-4.patch, failed testing. View results

Zenovii_Kholyavka made their first commit to this issue’s fork.

Zenovii_Kholyavka opened merge request !14

Zenovii_Kholyavka’s picture

Zenovii_Kholyavka CreditAttribution: Zenovii_Kholyavka at EPAM Systems commented
Status: Needs work » Needs review

I fixed test related translations and added changes which was added by @mrinalini9. Please review.

Andrii Kleba’s picture

Andrii Kleba CreditAttribution: Andrii Kleba at EPAM Systems commented
Status: Needs review » Reviewed & tested by the community

@Zenovii_Kholyvka works as expected, thanks

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T commented

I've checked and it really works well. Thank you so much

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T commented
Status: Reviewed & tested by the community » Fixed

Moved to the dev branch.

Thank you so much for your contribution

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented

Why did these translations need context on them? Each word is pretty generic, I wouldn't think it would be necessary.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented

Following on https://www.drupal.org/docs/7/api/localization-api/string-context I don't see why the context is necessary. Maybe "always" but even then I'm doubtful.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented

I removed the contexts for most of these and updated the context for "always" to "At all times" and for "never" to "At no time" (also never was missing translation). Also the sitemap generation needs an untranslated string at all times, so I needed to add a parameter to return the raw string value instead of the translated string.

lonalore’s picture

lonalore
Primary language Hungarian
Location Budapest, Hungary
CreditAttribution: lonalore as a volunteer commented

I downgraded the module back to v1.2 on my websites because Google invalidated all URLs due to (bad hungarian?) translation. Please add a context to all of the changefreq translations, or please provide an option to disable translation.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented

@lonalore You're not on the latest release, this was fixed in the 1.3 release.

lonalore’s picture

lonalore
Primary language Hungarian
Location Budapest, Hungary
CreditAttribution: lonalore as a volunteer commented

I upgraded the module to v1.3 again, and I get invalid sitemap.xml now.

Module:

# Information added by Drupal.org packaging script on 2022-07-20
version: '8.x-1.3'
project: 'xmlsitemap'
datestamp: 1658327439

Generated sitemap:
https://www.erettsegitetel.hu/sitemap.xml

As I see it, the bug may be that the xmlsitemap_get_changefreq() function is called with 1 parameter everywhere, and the second parameter is TRUE by default, so changefreq is translated in all cases.

lonalore’s picture

lonalore
Primary language Hungarian
Location Budapest, Hungary
CreditAttribution: lonalore as a volunteer commented
FileSize
Képernyőkép_2022-09-20_09-30-31.png38.57 KB

Google Search Console says that all links are invalid due to the changefreq.

Google Search Console

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot for Principal Financial Group commented
FileSize
2850367-fix-changefreq-untranslated-default.patch2.12 KB

Yup you're right, I shouldn't have changed the default option for xmlsitemap_get_changefreq_options() to return the translated string by default, it should return the untranslated string by default. Patch fixes the issue and has test coverage.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot for Principal Financial Group commented
Status: Closed (fixed) » Needs review

  • Dave Reid committed d8968af on 8.x-1.x 
    Issue #2850367 by Dave Reid: Fixed xmlsitemap_get_changefreq() should...
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot for Principal Financial Group commented
Status: Needs review » Fixed

Committed #21 to 8.x-1.x and will release today or tomorrow.

  • Dave Reid committed d8968af on 2.x 
    Issue #2850367 by Dave Reid: Fixed xmlsitemap_get_changefreq() should...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.