dougwollison / nlingual

Versatile and flexible multilingual system for WordPress.
https://wordpress.org/plugins/nlingual/
GNU General Public License v2.0
13 stars 7 forks source link

Settings conflict prevents automatic language redirection #12

Closed notabeatle closed 4 years ago

notabeatle commented 4 years ago

Checking "URLs for the default language will be unmodified" prevents redirection to user agent's preferred language. Unchecking it causes redirection to start working, but of course means the default language also gets redirected. Fiddling with other maybe-relevant settings like "Should the language of the requested post take precedence in the event of a language mismatch?" doesn't fix the behavior.

I've reproduced this in a basic docker-compose set-up of Wordpress with just nLingual installed, by creating a translation of the initial "Hello world!" post, getting redirection from the home page to work when my browser language is set to that of the translation, then breaking redirection by checking the "URLs for the default language will be unmodified" box.

This seems like a bug to me in that it's preventing functionality I both wanted and expected, given the settings I'd chosen, to work, but is that the case, or is it in fact working as intended?

notabeatle commented 4 years ago

Dropped in a PR for changes that make this behave the way I expected it to. I tried some other things to see if it broke anything, and didn't find any issues, but of course didn't try every possible combination of settings. Notably I'm not set up to easily test that I didn't mess up subdirectory-based translation redirection, somehow, though I think I've not done anything that should affect that. Didn't see any automated tests to run so I just poked at things I thought might have been messed up by these changes, and didn't find anything.

dougwollison commented 4 years ago

Will take a look; been juggling a few too many rush projects at work.

It is indeed an issue but it's also arguably a limitation that should be expressed better by the settings. I don't want to prevent someone who's browser language is, say, french, from being able to view the site in english for whatever reason. This could always be overwritten with a GET arg on the URLs of course. I may have even fixed this partially on one of the other branches I've yet to merge (recent development has been a bit... chaotic).

Until I'm able to publish a semi-thorough patch, you should be able to use the available hooks to make the changes you want.

notabeatle commented 4 years ago

Cool, thanks for the response! More details about the exact issue in the PR, let me know if you have any questions about it. Meanwhile, I'll look at a hook-based workaround, as suggested.

dougwollison commented 4 years ago

Hey, finally had some time to look at this in detail, working on a patch soon that should solve this sanely.

Basically, I've rewritten the handling of skip_default_l10n to not apply when the browser's language matches a non-default supported language. This allows, for example, a french visitor to be redirected to the french page (unless post_language_override is set, it takes precedence), but if they want to they can view the english pages by visting URLs with /en/ specified (or en.* in the case of subdomain handling... which come to think of it I almost never test).

I'm not sure that made sense so here's an outline of scenarios for an english/french site:

Visitor Language Requested URL Redirected URL
English / /
French / /fr/
English /en/ /
French /en/ /en/
English /about/ /about/
French /about/ /fr/a-propos/
English /en/about/ /about/
French /en/about/ /en/about/

I've gotta test it on a few dev sites to make sure first. Should have this ironed out into a proper update next week, provided a certain project doesn't spontaneously return from limbo.