WPML Rewrites defaulting to last language prefix found

[shaneonabike]

Based on dev/wordpress#153

Description

Recently, we have been having a problem with the base CiviCRM page not rendering paths like event/info/... properly except for the translated pages.

After further research, I found that the rewrites were defaulting to the page id of the translated CiviCRM page.

Rewrite path	Rewrite	Source
^civicrm/([^?]*)?	index.php?page_id=9439&civiwp=CiviCRM&q=civicrm%2F$matches	other
	[1]
^fr/civicrm/([^?]*)?	index.php?page_id=9439&civiwp=CiviCRM&q=civicrm%2F$matches	other
	[1]

The first path ^civicrm/([^?]*)? should be pointing to the Base Page not the actual translated page.

Setup

• English Canada (default)
• French Canada
• CiviCRM Languages added (as above)
• Mapping codes above to CiviCRM language

After

Essentially, what this does is not to add the language translation redirect to the basepage ID and vic versa. In my scenario this is totally messing with the translation of CiviCRM and causing a page not found on the translated link.

Specifically

 $collected_rewrites[$post_id][] = $regex_path;

and also for the other languages

 $collected_rewrites[$basepage->ID][] = $regex_path_converted;

[shaneonabike]

FWIW before we actually apply this we should check a few other scenarios of site configurations (WPML has a few different types of settings). I don't forsee if affecting those since it should work, but better to check. I just didn't have time to test them

[seamuslee001]

@shaneonabike is this where you have the primary locale in the url or not?

[kcristiano]

The patch looks sensible, but I don't have the necessary WPML/Polylang licenses or setup to test. If anyone can setup and test it would be appreciated.

[christianwach]

@shaneonabike Okay, I can confirm your issue but only in certain circumstances.

Firstly, it looks to me as though this only happens when not using "Use directory for default language" as per the screenshot below.

Secondly, it seems that CiviCRM has to have been installed in a language that is no longer the default for the problem to occur... or at least that's what I'm finding. And furthermore I cannot actually change the Basepage to the one I want to make things work again.

A possible reason for the problem is that the Basepage ID isn't quite what CiviCRM thinks it is, since it uses $basepage = get_page_by_path($config->wpBasePage) to retrieve it. If you switch default languages around in WPML, then $basepage->ID never seems to match $post_id and then I see the rendering problem.

So it looks to me like we need to rethink the way in which CiviCRM handles identifying the Basepage - i.e. it should store the ID not the path. And FWIW, the code in this PR doesn't solve that.

In summary, I think there are deeper problems than the one which this PR addresses.

[christianwach]

If you switch default languages around in WPML, then $basepage->ID never seems to match $post_id and then I see the rendering problem.

Correction to the above: I think I see the problem when the default language is the one which the Basepage is set to. If I switch the default language to one that isn't the same as the Basepage is set to, then everything works as expected with the current code.

I think a way forward might be to use the civicrm/basepage filter... but this really is can of proverbial worms.

[shaneonabike]

@christianwach Yes we are using Different langauges in directories as you suggested above.

Secondly, it seems that CiviCRM has to have been installed in a language that is no longer the default for the problem to occur... or at least that's what I'm finding.

Can you elaborate on what you mean here? This site has been around for quite some time, and didn't do anything unusual. We have:

• Site Languages: English (default), French
• CiviCRM Languages: English (Canada), French (Canada)
• Default CiviCRM page, but it is crucial that you translate the CiviCRM page to the other language
• PHP 8.2
• Mapping of those languages to CiviCRM (see below)

• Proper mapping in CiviCRM settings (or wp-config.php)
• Perhaps a gotcha is that this is installed on a multisite, but it's the main site that is maintaining WPML and also CiviCRM so it shouldn't be a factor really.

define('CIVICRM_LANGUAGE_MAPPING_EN', 'en_CA');
define('CIVICRM_LANGUAGE_MAPPING_FR', 'fr_CA');

In my output I verified the IDs and they match properly both the post_id and the basePage. What I was reporting in my original issue https://lab.civicrm.org/dev/wordpress/-/issues/153 is that essentially when the system goes to render the rewrites it ends up prioritizing the basePage for all languages.

2025-03-07 17:29:16+0000  [warning] Rewrites array (
  4500 => 
  array (
    0 => 'civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
  ),
  9439 => 
  array (
    0 => 'fr/civicrm/',
  ),
)

2025-03-07 17:29:16+0000  [warning] Second round array (
  4500 => 
  array (
    0 => 'civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
    3 => 'civicrm/',
    4 => 'civicrm/',
    5 => 'civicrm/',
    6 => 'civicrm/',
    7 => 'civicrm/',
    8 => 'fr/civicrm/',
  ),
  9439 => 
  array (
    0 => 'fr/civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
  ),
)

What is happening is that after the array unique basically the rewrites are created for 4500 to be the path for everything (all languages). Thus when I go to say https://myorg.org/fr/civicrm/event/register/?reset=1&id=69 it will say Access Denied or Not Found because it's obviously an incorrect redirect (you can't show the French language for an English basepage).

Is that clearer maybe?

[shaneonabike]

If I switch the default language to one that isn't the same as the Basepage is set to, then everything works as expected with the current code.

So to reiterate what you are saying here (it took me a few runs to get my head around it)

• Default language: English
• Basepage (default): French

Did you also translate the basepage? If that's the case I'm happy to test that scenario also? But also if that IS the case then maybe we have to reverse the code somewhat to "help it along"? I don't think I Have an edge case setup here..

Thoughts?

[shaneonabike]

I just wanted to reloop on this to see if there wouldn't be a way to potentially integrate this. I don't think that it should generate so many duplicate paths as it is, and clearly it seems wrong (to me at least) that the translation path appears on the basepage of the other language. That is what my patch is attempting to address.

[christianwach]

So it looks to me like we need to rethink the way in which CiviCRM handles identifying the Basepage - i.e. it should store the ID not the path.

@shaneonabike I think the blocker is the problem I stated here. I'd like to see some work done on addressing this before returning to this WPML issue. Perhaps as simple as getting a new setting for the Basepage ID into CiviCRM Core. That would give us a chance to fix this properly.

[shaneonabike]

hey @christianwach hope you are well! We already have that with CiviCRM Admin tools so could their be a linkage via that OR are you implying that we really need something in core that would take care of this? If so, can you point to me where this is actually getting set in the first place. I don't think I understand how core is aware at all which is the correct basePage (besides obviously it's initial creation of the page, but then if someone deletes it and recreates a new one).

My gutt says it just searches for /civicrm but who knows

[christianwach]

I don't think I understand how core is aware at all which is the correct basePage (besides obviously it's initial creation of the page, but then if someone deletes it and recreates a new one).

@shaneonabike It's non-obvious how to get a new setting into Core. I had to have help for wpLoadPhp when that was introduced. Looks like specifying it in that file is the start of the process.

[JoeMurray]

@eileenmcnaughton is a key person to gatekeep having too many settings in CiviCRM / agreeing to a new one. A provisional approach sometimes sees them created as PHP variables in civicrm.settings.php and not in the UI.

[eileenmcnaughton]

So to articulate the considerations when adding settings to core

1. they can be hard to maintain - this applies in particular to new functionality added to core and 'hidden' behind a setting. For example someone added a setting which changes civicrm_email.on_hold to a ternery, if the setting is enabled. Something changed. No-one knew or thought to see the impact with the setting enabled and it broke/ regressed. Often this is what we are trying to prevent - sneaking functionality in behind a setting that will create a maintenance burden down the track. In some cases when we let one of these settings in we wind up doing weeks of work later to re-write the feature to be maintainable.

2. they can allow users to break their sites / create security concerns. If getting the setting wrong could cause major issues it is probably an argument for something less accessible like a define in civicrm.settings.php

3. they can be confusing - ever looked at the Display Settings page???

4. they can be a way to get a half-baked or bad approach into core and a proposed setting is often a clue the approach is wrong (this is a variant on 1)

If @christianwach & @kcristiano consider a setting makes sense, given the above, then I think they understand this particular PR better than I do.

With regards to how to add them - @christianwach is right to give civicrm/civicrm-core@fdd770d as an example - in most cases you will also want to have a key like

'settings_pages' => ['wordpress' => ['weight' => 150]], - that should be enough to make the setting accessible by the url civicrm/admin/setting/wordpress - provided there is a Menu item and (optionally) a Navigation item. You can also add a setting to an existing page but refer 3 above

[christianwach]

@eileenmcnaughton Thanks for detailing the issue with settings in Core.

My suggestion of a new setting is aimed at eventually deprecating and removing the current wpBasePage setting in favour of a new wpBasePageID setting of 'type' => 'Integer' that does not suffer from the ambiguity of the current wpBasePage setting that I detail above:

A possible reason for the problem is that the Basepage ID isn't quite what CiviCRM thinks it is, since it uses $basepage = get_page_by_path($config->wpBasePage) to retrieve it.

This query to retrieve the Basepage fails in certain circumstances in a WPML context - whereas calling $basepage = get_post($config->wpBasePageID) would not.

I'm open to the idea of deprecating the Core wpBasePage setting in favour of one managed by the code in this CiviCRM-WordPress repo. There are already a number of settings managed here, e.g. civicrm_shortcode_mode. This would, however, involve changes here and in Core - and in any Extensions that may happen to use $config->wpBasePage.

[shaneonabike]

Also, I think that this behaviour is already kind of overwritten or optional via the CiviCRM Admin Utils so I suspect that people are somewhat accustomed to it. I think it's a good move because we are having lots of problems related to the paths that potentially could be resolved

[christianwach]

I think that this behaviour is already kind of overwritten or optional via the CiviCRM Admin Utils

@shaneonabike Not sure what you mean here. Can you explain?

[shaneonabike]

@christianwach Maybe it isn't CiviCRM Admin Utilities but basically we have the Wordpress Base Page option to select as shown below. But I don't know if that is in the WP CiviCRM core. I would assume this is storing the actual Page ID.

[christianwach]

@shaneonabike Yes, that's shown on the Settings page, but the setting itself is held in CiviCRM Core and is the Page path not its ID.

[shaneonabike]

Oh boy! 😿 Ok well there we have it .. wouldn't this just be a matter of changing the code over to use the Page ID instead of the actual path. Therefore, not a mega change because realistically when we need the actual page path we could do a look up with Wordpress. IMHO this is better because if down the road someone changes the page path it will always be correct.

[christianwach]

Therefore, not a mega change

@shaneonabike If only it were simple, but there's a fair bit of code in Core that uses the current setting:

https://github.com/search?q=repo%3Acivicrm%2Fcivicrm-core%20wpBasePage&type=code

sigh

[shaneonabike]

I want to loop back on this because I have been receiving comments about this from people. For now I have patched this on several larger distributions of sites we have and it's working. My approach was to basically reduce down the permalink structure since it is repeating (thus confusing the system) with duplicates.

I am happy to work with Core and yourselves to find a solution to remove the Page ID or whatever other solution we have in the future, but we should probably make this functional for people now in the meantime right?

cc @christianwach @kcristiano

[christianwach]

@shaneonabike I don't have the resources to implement the reworking of the path ==> id storage at the moment. But if you wrap your alternative code in a filter with the default being existing behaviour, I'd be open to merging what you've done here. I don't know about @kcristiano but we will meet later to discuss.

[shaneonabike]

@christianwach So are you suggesting that rather than proposing my fix you want me to implement a hook so that it looks like this

1. Existing behaviour to create permalinks
2. Call hook
3. Hook (if setup externally modified permalinks)
4. Permalinks are set

I could do that but it still won't resolve the issue that some people are having with regards to WPML because it would rely on them to implement a solution to resolve it. We ultimately want this to work out of the box right?

Primary what I was trying to do to reduce the permalinks that were duplicated. I posted above, but will post here again.

Putting both languages associated to the same Post ID is wrong because it confuses WP. What I did in the refactor is essentially to not do that. Stick to the actual language we are in.

2025-03-07 17:29:16+0000  [warning] Rewrites array (
  4500 => 
  array (
    0 => 'civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
  ),
  9439 => 
  array (
    0 => 'fr/civicrm/',
  ),
)

2025-03-07 17:29:16+0000  [warning] Second round array (
  4500 => 
  array (
    0 => 'civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
    3 => 'civicrm/',
    4 => 'civicrm/',
    5 => 'civicrm/',
    6 => 'civicrm/',
    7 => 'civicrm/',
    8 => 'fr/civicrm/',
  ),
  9439 => 
  array (
    0 => 'fr/civicrm/',
    1 => 'civicrm/',
    2 => 'fr/civicrm/',
  ),
)

But correct me if I'm wrong cause I could have misunderstood. I really appreciate your time and guidance and just want to make something that works for the community.

[shaneonabike]

Also, as a side note it could be good for someone (I could) to reach out to WPML to see if they wanted to basically work with us on this. They would have resources to do so and it's in their interest to make things function.

[kcristiano]

@shaneonabike my issue is I have no ability to test this PR. We currently have no one on WPML. Other WP Translation plugins yes but not WPML.

A testing site that is configured would also be a big help.

For that reason if we could provide the necessary hooks via an extension/plugin I'd be more comfortable. If they can install translations, They could install an extension.

Asking WPML for help is a good idea and cannot hurt.

[christianwach]

We ultimately want this to work out of the box right?

@shaneonabike I've said this previously - get_page_by_path( $config->wpBasePage ) is the heart of the problem here. WPML is simply making the problem visible because it returns a different Base Page to the one that is actually wanted.

This PR may paper over the cracks in your "Language URL format" scenario but doesn't seem to be tested in other "Language URL format" scenarios and could therefore break sites that don't have your particular setup.

Anyone wanting to replace the behaviour in this class can do that with the existing code, e.g.

remove_action('civicrm_after_rewrite_rules', [civi_wp()->compat->wpml, 'rewrite_rules']);
add_action('civicrm_after_rewrite_rules', 'my_wpml_rewrite_rules', 10, 2);
function my_wpml_rewrite_rules($flush_rewrite_rules, $basepage) {
  // Your logic here.
}

Perhaps a matrix of scenarios with outcomes (e.g. this one) with your code and the existing code might help?