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

[SLUGS] fix language autodetection #1364

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/init.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ function qtranxf_init_language(): void {
$url_info['language'] = qtranxf_detect_language( $url_info );
$q_config['language'] = apply_filters( 'qtranslate_language', $url_info['language'], $url_info );

QTX_Module_Loader::load_active_modules_early_hooks();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing the loading sequence logic, as we currently load the code of the modules later.
I don't think we should make it more complex by having "early hooks" and "later hooks". The flow is already complicated and I've been trying to simplify it with consistent loaders.
Also there's some rework to do for #1325 with mu-plugins.
But I understand the need for some plugins that have effect on redirection step that comes quite early on.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now no requested change, but I keep it on hold a bit. I still may merge the patch as it is and refactor the loading sequence completely from this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point I'm not convinced we should generalize this pattern to all modules.
The Slugs module is quite special because it touches some core functionalities.
For now I suggest to have a special case for slugs in that section. I'll try to push another patch.

assert( isset( $q_config['url_info']['doing_front_end'] ) );
if ( $q_config['url_info']['doing_front_end'] && qtranxf_can_redirect() ) {
qtranxf_check_url_maybe_redirect( $url_info );
Expand Down
20 changes: 20 additions & 0 deletions src/modules/module_loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,24 @@ public static function load_active_modules(): void {
}
}
}

/**
* Loads early hooks from modules previously activated in the options.
*
* Attention! This assumes the current states stored in the options are valid.
* This doesn't perform any check, neither on the plugin conditions nor the folder structure.
* In the worst case the state can be refreshed by reactivating the plugin.
*
* Note also the modules should be loaded before "qtranslate_init_language" is triggered.
*/
public static function load_active_modules_early_hooks(): void {
$modules_state = get_option( QTX_OPTIONS_MODULES_STATE, array() );

foreach ( $modules_state as $module_id => $state ) {
$target = QTRANSLATE_DIR . '/src/modules/' . $module_id . '/' . 'early_hooks.php';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for early_hooks.php this is not different than load_active_modules for the main loader.php.
We need a better design with a unique entry point.

if ( $state === QTX_MODULE_STATE_ACTIVE && file_exists( $target ) ) {
require_once( $target );
}
}
}
}
31 changes: 31 additions & 0 deletions src/modules/slugs/early_hooks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

add_filter( 'qtranslate_language_detect_redirect', 'qtranxf_slugs_language_detect_redirect', 600, 3 );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to avoid using a filter for an internal mechanism, that should be called explicitly to have full control on the flow. I don't think any other plugin should have a possibility to alter this behavior between core and the slugs module (once it's enabled).


/**
* Disables default redirection when language is not detectable from url (e.g. default language url with hide_default_language activated).
* Exception is made for site_url(), for which language detection feature is preserved.
*
*
* @see qtranxf_check_url_maybe_redirect
* @param string $url_lang proposed target URL for the active language to redirect to.
* @param string $url_orig original URL supplied to browser, which needs to be standardized.
* @param array $url_info a hash of various information parsed from original URL, cookies and other site configuration. The key names should be self-explanatory.
*
* @return string resulting redirection url
*/
function qtranxf_slugs_language_detect_redirect($url_lang, $url_orig, $url_info): string {
global $q_config;
/* Make sure urls with no lang info are treated as default language, unless it's site_url */
if ( untrailingslashit( site_url() ) != untrailingslashit( $url_orig ) && empty( $url_info['lang_url'] ) ){
return qtranxf_convertURL( $url_orig, $q_config['default_language'], false, true );
/* The following fixes the case when the browser removes language marker from default language site url (caching?) passing directly _$SERVER['REQUEST_URI'] with language info removed.
* In this case is not possible to switch to default language from site url.
* Application of this hack is narrowed down to the cases where referer is site url in current language, to preserve language detection feature as much as possible.
* @TODO: check if a cleaner fix is applicable. */
} else if ( $q_config['hide_default_language'] && untrailingslashit( site_url() ) == untrailingslashit( $url_orig ) && untrailingslashit($url_lang) === untrailingslashit( wp_get_raw_referer() ) ) {
return qtranxf_convertURL( $url_orig, $q_config['default_language'], false, true );
}
herrvigg marked this conversation as resolved.
Show resolved Hide resolved
/* All other cases follow default behaviour */
return $url_lang;
}