This issue has been preventing me from upgrading from a commit around October when domain stopped working with a lot of our custom multilingual work.

It took some time to finally track it down but it appears to be that the domain source module's outbound path processor gets called far too early and $options['language'] is never actually set.

The problem this causes is that on a multilingual site, when viewing an entity that has a few translations, it always defaults to looking at its default translation. If that default translation is for a different domain than the endpoint you want to look at, it redirects over to a completely different domain.

When I bumped the priority down to 90, I had to fix a few other errors that came screaming out, but I hope this works. I don't know the original reason that the priority was 200.

CommentFileSizeAuthor
#4 2973694-domain_outbound-path-processor-no-language_4.patch1.98 KBAnonymous (not verified)
#2 2973694-domain_outbound-path-processor-no-language.patch2.13 KBAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented

vilepickle created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 2973694-domain_outbound-path-processor-no-language.patch2.13 KB
Anonymous’s picture

Anonymous (not verified) commented
Title: Outbound path processor does not have access to language and redirects » Outbound path processor does not have access to language and then redirects
Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2973694-domain_outbound-path-processor-no-language_4.patch1.98 KB

Did some testing by actually implementing hook_domain_source_path_alter and realized source is passed by reference. This should fix it.

Btw for anyone passing by, implementing the following hook allows me to control paths that are non-entities to make sure the user is on the correct domain based on a language code in the path. This is important for us so we're not able to view French content on say, the US domain. I implemented a 'master request data' class in my custom module that knows what the requested language is, but it basically explodes the current path and finds the language code prefix.

/**
 * Implements hook_domain_source_path_alter().
 *
 * Make sure we're on the correct domain based on the requested
 * path language.
 */
function domain_locale_custom_domain_source_path_alter(&$source, $path, $options) {
  $data = \Drupal::getContainer()->get('domain_locale_custom.master_request_data');
  
  if ($data->requestedLangcode() == 'en-ca' || $data->requestedLangcode() == 'fr-ca') {
    $options['active_domain'] = \Drupal::service('entity_type.manager')->getStorage('domain')->load('gfs_ca');
  }
  if ($data->requestedLangcode() == 'en-us') {
    $options['active_domain'] = \Drupal::service('entity_type.manager')->getStorage('domain')->load('gfs_com');
  }
  $source = $options['active_domain'];
}
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Is this corrected by #2940296: Combination of language negotiation and path aliasing can cause a corrupted route cache, 404s or is it a different issue?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

It was originally 200 because no guidance is given for setting priority in the subscriber documentation ;-).

Anonymous’s picture

Anonymous (not verified) commented

That issue does seem somewhat related... However it's odd that at weight 90 language becomes available compared to the current 200 weight. The linked issue makes it seem like language never gets set. But maybe it just never gets set when Domain itself needs it and something else does at a different time.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

One change in the patch: We shouldn't set $source before calling the alter hook. Doing so rewrites the url to absolute, even if nothing alters the $source element. It would also be an API change.

    // One for other, because the latter is resource-intensive.
    else {
      $this->moduleHandler->alter('domain_source_path', $source, $path, $options);
    }
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

That change broke the DomainSourceExcludeTest by forcing all URLs to be absolute, which we don't want.

Anonymous’s picture

Anonymous (not verified) commented

Ah I see. Is the way around that to send active_domain to the alter hook as source? It seems like that might not work though, haven't tested it out.

I will get a chance to re-look at it in the next couple days.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Well, as documented, we send NULL.

From the API point of view, the assumption is that NULL means "no source has been set, use the active domain" so passing a value seems redundant. The caller is expected to know if the URL has a canonical domain and respond accordingly.

In the case of entities, we only provide a canonical URL if the domain source field is populated.

Are you effectively saying that, as a module developer, you cannot use the hook if NULL is passed?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Fixed

This has been committed in modified form as noted.

Status: Fixed » Closed (fixed)

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