/**
 * 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

Comments

Cyberwolf created an issue. See original summary.

cyberwolf’s picture

cyberwolf commented
StatusFileSize
new xmlsitemap_change_frequency_translatable-2850367-2.patch1.23 KB
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
mrinalini9’s picture

mrinalini9 commented
StatusFileSize
new 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 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 commented
Status: Needs review » Reviewed & tested by the community

@Zenovii_Kholyvka works as expected, thanks

renatog’s picture

renatog commented

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

renatog’s picture

renatog 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
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
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
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
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
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
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
commented
StatusFileSize
new 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
commented
StatusFileSize
new 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
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
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.