Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translated Nodes do not switch languages properly when set as the home page. #4785

Open
jenlampton opened this issue Dec 6, 2020 · 26 comments · May be fixed by backdrop/backdrop#3433
Open

Comments

@jenlampton
Copy link
Member

jenlampton commented Dec 6, 2020

Description of the bug

Translated Nodes do not switch languages properly when they are set as the front page.

If I create an English about page, then translate into Spanish, I can click the "English" link to see the English version, or the "Spanish" link to see the Spanish version -- no problem.

However, if I then navigate to admin/config/system/site-information and set my Default home page to be about that breaks the language switching.

Now when I go to the about page (used for home) and click the "Spanish" link to see the Spanish version of the page, the page remains in English. The only way I can view the Spanish version of the page is to navigate directly to the Spanish node.

Steps To Reproduce

  1. Enable locale and content translation modules
  2. add at least 1 other language
  3. Translate the about page
  4. Place the Locale language switcher block.
  5. Navigate to admin/config/system/site-information and set the Default home page to be about
  6. Visit the about/home page.
  7. Use the link in the language switcher block to view the other version of the home page.

Actual behavior

Front page remains in primary language, regardless of which language was selected.

Expected behavior

Front page should show in selected language.

@jenlampton
Copy link
Member Author

Well, I'm not sure what fixed it but my site seems to be switching languages properly now. (I know edited the both translations of the content and both menu entries. Not sure what else...)

@klonos
Copy link
Member

klonos commented Dec 6, 2020

Related: I'm not sure what has changed recently, but I've been often testing PRs with RTL languages by doing the following:

  • enable Language module
  • add Arabic, or Hebrew, or any other RTL language
  • set the RTL language as the default -> site (front end and admin UI) instantly changes to RTL so that I can test 👍

This doesn't seem to work as before anymore ("before" being 2-3 bug releases back maybe?), in that I now have to specifically add the language code in the URL to have things switch to RTL.

@klonos
Copy link
Member

klonos commented Dec 6, 2020

...scratch all the above; I just tested again and it seems that we've fixed that in the latest 1.x. Everything works as expected again 👍

@jenlampton
Copy link
Member Author

jenlampton commented Dec 6, 2020

I wonder if something recently caused an extra cache-clear to be necessary, when it wasn't before? (Maybe entity cache? No, that wouldn't make sense, these are different entities...)

@jenlampton jenlampton reopened this Dec 8, 2020
@jenlampton
Copy link
Member Author

jenlampton commented Dec 8, 2020

Well, everything stopped working as expected again on my site. Re-opening with new STR (in OP).
It appears as though is is also a known drupal issue: https://www.drupal.org/project/drupal/issues/339958

@indigoxela
Copy link
Member

Well, everything stopped working as expected again on my site

@jenlampton A quick question: I suppose you do have the Redirect module turned on? Did you change the title (and/or alias) of the node between the time it seemed to work and the time it stopped working?

Anyway, I can confirm that the language switcher does not switch the node translation if the node is set to be the front page.
For other nodes and their translations everything works fine.

@jenlampton
Copy link
Member Author

I'm not sure where I need to fix this problem in core, but I've added a work-around to my site as follows:

/**
 * Implements hook_url_inbound_alter().
 */
function HOOK_url_inbound_alter(&$path, $original_path, $path_language) {
  if (backdrop_is_front_page()) {
    global $language;
    if ($language->langcode != $path_language) {
      if (arg(0) == 'node' && is_numeric(arg(1))) {
        $node = node_load(arg(1));
        $translations = translation_node_get_translations($node->tnid);
        if (!empty($translations[$language->langcode])) {
          $path = 'node/' . $translations[$language->langcode]->nid;
        }
      }
    }
  }
}

@indigoxela
Copy link
Member

I'm not sure where I need to fix this problem in core

I'm also clueless. 🤷

Maybe something in translation_language_switch_links_alter fails?

Or the config item "site_frontpage" overrides it again at a later point?

@jenlampton
Copy link
Member Author

jenlampton commented Dec 9, 2020

Maybe something in translation_language_switch_links_alter fails?

I suspect it is actually something in how it determines what to load for the home page. When my site is on "/es" it always loads the English version of the home page instead of the Spanish, because that is what's set in system.core.site_frontpage.

What it needs to do is retrieve site_frontpage, then check the language, then check if there is a translation of the site_frontpage in the matching language, and if so, load that instead.

I spent some time poking around in path.inc and system.module but was unable to find where the frontpage magic happens... My next step is to search for drupal issues relating to site_frontpage and review the patches. Then I'll see if there's a hook nearby that lets me change that behavior, and then maybe add that into content translation module?

@indigoxela
Copy link
Member

indigoxela commented Dec 10, 2020

I suspect it is actually something in how it determines what to load for the home page

Seems plausible. Then it's rather a bug in the translation module, which fails to load the proper nid for the current tnid?

This is also broken in Drupal 7, BTW. Fancy that I never realized that.

@indigoxela
Copy link
Member

Hey, @jenlampton, you know what... your "workaround" implemented as translation_url_inbound_alter() (use the alter hook in the translation module) fixes the problem. 😉

Mind to create a PR from it? Or are there any concerns?

@jenlampton
Copy link
Member Author

I've created a PR. My only concern was if adding a node_load() here would degrade performance, but I think entitycache should prevent that from doing any additional db queries.

@docwilmot
Copy link
Contributor

Posting mostly to sort out in my head: the process goes roughly bootstrap loads the saved front page from config to $_GET['q'], then when menu_execute_active_handler() is called, that checks menu_get_item() which loads the $path as whatever is saved to $_GET['q']. Then Layout takes over (because the active handler in Backdrop is layout_route_handler()).

See:

function backdrop_path_initialize() {
  // Ensure $_GET['q'] is set before calling backdrop_get_normal_path(), to
  // support path caching with hook_url_inbound_alter().
  if (empty($_GET['q'])) {
    $_GET['q'] = config_get('system.core', 'site_frontpage');
  }
  $_GET['q'] = backdrop_get_normal_path($_GET['q']);
}

Somehow I dont see where the translation system gets involved here at all.

So hook_url_inbound_alter() is actually called in backdrop_get_normal_path(); so maybe we can add a call to menu_get_object() there instead of the node load, and trigger the translation action right into that function rather than an alter()?

@indigoxela
Copy link
Member

indigoxela commented Dec 11, 2020

This is how I tested:

  • Enable language, translation, locale modules
  • Add a language
  • Set page multilingual support to enabled, with translation
  • Add a page and translate
  • Set that node (one of them) as the start page (default home page)
  • Add the language switcher to default layout
  • Go to start page and switch language

Works like a charm.

@docwilmot did you miss that there's already a PR?

@indigoxela
Copy link
Member

indigoxela commented Dec 11, 2020

@jenlampton mind to add "Fixes" to your PR comment? It's much easier to find if the link, if it appears here in the sticky header. 😉

@docwilmot
Copy link
Contributor

@docwilmot did you miss that there's already a PR?

No, the line about "So hook_url_inbound_alter() is actually called in backdrop_get_normal_path(); so maybe ..." is with reference to that PR. We usually try to directly fix the origin of the process, rather than calling alters. Ideally altering should be for contrib, or if other modules are accessing a core API, I think.

@indigoxela
Copy link
Member

We usually try to directly fix the origin of the process, rather than calling alters

IMO it has to be fixed in the translation module, translations are only available, when it's enabled. I don't think that menu_get_object() is the right place - why should it deal with tnid?

@docwilmot
Copy link
Contributor

I was considering avoiding the node load.

@indigoxela
Copy link
Member

I was considering avoiding the node load.

I see. But I don't think it does any harm.

@jenlampton jenlampton modified the milestones: 1.22.1, 1.22.2, 1.22.3 Jul 20, 2022
@jenlampton jenlampton modified the milestones: 1.22.3, 1.23.1 Sep 15, 2022
@jenlampton jenlampton modified the milestones: 1.23.1, 1.23.2 Dec 2, 2022
@quicksketch quicksketch modified the milestones: 1.23.2, 1.24.1 Jan 15, 2023
@jenlampton jenlampton modified the milestones: 1.24.1, 1.24.2 Mar 15, 2023
@jenlampton jenlampton modified the milestones: 1.24.2, 1.24.3 Apr 19, 2023
@klonos klonos modified the milestones: 1.24.3, 1.25.1 Jun 6, 2023
@quicksketch quicksketch modified the milestones: 1.25.1, 1.25.2 Jun 7, 2023
@quicksketch quicksketch modified the milestones: 1.25.2, 1.26.1 Sep 15, 2023
@quicksketch quicksketch modified the milestones: 1.26.1, 1.26.2 Oct 6, 2023
@quicksketch quicksketch modified the milestones: 1.26.2, 1.26.3 Dec 1, 2023
@quicksketch quicksketch modified the milestones: 1.26.3, 1.26.4 Dec 20, 2023
@quicksketch quicksketch modified the milestones: 1.26.4, 1.27.1 Jan 16, 2024
@quicksketch quicksketch modified the milestones: 1.27.1, 1.27.2 Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment