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

A dynamic rule based on 'en' language takes priority over all other languages #2043

Closed
morphinestyle opened this issue Mar 7, 2024 · 10 comments · Fixed by #2045
Closed

A dynamic rule based on 'en' language takes priority over all other languages #2043

morphinestyle opened this issue Mar 7, 2024 · 10 comments · Fixed by #2045
Labels
Milestone

Comments

@morphinestyle
Copy link

Shlink version

4.0.0

PHP version

8.2.0

How do you serve Shlink

Other (explain in summary)

Database engine

MariaDB

Database version

10.6

Current behavior

When I insert a dynamic rule with the 'en' language, this rule takes priority over all the others.

Expected behavior

The rule based on language works correctly regardless of the presence of a dynamic rule with 'en' language.

How to reproduce

My default language is ITALIAN, and in the Web UI, there's a link to a page in Italian.
Then I added a French page, a German one, and a Spanish one: each with its respective dynamic rule based on 'fr', 'de', and 'es'.
Everything works correctly until I add dynamic rules with 'en' language.
After inserting a dynamic rule based on language that contains 'en', Shlink no longer works correctly and redirects me to the 'en' rule regardless.

@acelaya
Copy link
Member

acelaya commented Mar 7, 2024

Have you tried putting that rule the last one on the list?

Browsers usually send multiple accepted languages, as a fallback in case the requested page is not available in the default language.

Every time Shlink verifies a language condition, it just checks if the language set in the condition is among the accepted languages from the request, and English is very likely being sent by your browser.

You can check the Accept-Language header in order to verify this.

A more detailed explanation can be found in the docs, where it says "How granular is the language condition?".

@acelaya
Copy link
Member

acelaya commented Mar 7, 2024

Just to give you an example, this is the Accept-Language header my browser (Chrome) sends to any website I visit:

image

That means I prefer Spanish from Spain if available, then any Spanish, then English from US, then any English, and then for some reason, Japanese 🤷🏼‍♂️

Initially I decided to ignore the quality set for every language (the part after the q) and try to match all the languages on the list, but I'm thinking, in order to avoid unexpected matching, Shlink should ignore anything with a quality lower than 0.9

As a workaround, putting the English rule last should do the trick.

@acelaya acelaya added this to the 4.0.1 milestone Mar 7, 2024
@acelaya acelaya moved this to Todo in Shlink Mar 7, 2024
@morphinestyle
Copy link
Author

I will be away from the office until the weekend, I will get you the requested information as soon as possible.

Yes, I tried to move the priority of the 'en' rule but nothing changes: I don't remember if it was also tested as last, but if it is present it has priority over all of them regardless of the priority order set.

For the test I used Chrome and Firefox with specific extensions that I use to test multilingual on Joomla and Wordpress.

I will let you know the value of the Accepted-Language header as soon as possible.

@acelaya
Copy link
Member

acelaya commented Mar 7, 2024

I can see a slightly different behavior in Firefox, but restricting the rule to match languages with quality between 0.9 and 1 still would match on a more predictable way.

image

@morphinestyle
Copy link
Author

I did some very quick and quick tests:

Chrome seems to provide a more precise header:
Accept-Language: it-IT,it;q=0.9
which would seem to make Shlink work properly.

While Firefox provides a less precise header:
Accept-Language: it-IT,it;q=0.8,en-US;q=0.5,en;q=0.3
which makes Shlink's algorithm go haywire.

Obviously tested with native browsers WITHOUT EXTENSIONS THAT MODIFY THE ACCEPTED LANGUAGE HEADER.

I think with Firefox, setting the 'en' rule with the lowest priority on Shlink starts to work better.

Could Shlink's algorithm probably be improved so that it better considers the priority of the language with greater weight?

In any case, I managed to do these very quick tests, but before the weekend I won't be able to dedicate myself with the right attention.

@acelaya
Copy link
Member

acelaya commented Mar 7, 2024

Thanks for checking.

I actually put together a PR that should address this: #2045

It changes the logic so that Shlink does not try to match against fallback languages, only the "actual" ones, by discarding any language with a quality lower than 0.9.

So for example, for it-IT,it;q=0.9 it would check both it-IT and it, while for it-IT,it;q=0.8,en-US;q=0.5,en;q=0.3 it would evaluate only it-IT.

This effectively means only it or it-IT would throw a positive match, which is probably what one would expect.

@morphinestyle
Copy link
Author

In this/my case would it mean that Firefox will redirect to the main long URL skipping the dynamic 'it' rule?

Would it make sense to consider only the language with the highest 'q' value?

@acelaya
Copy link
Member

acelaya commented Mar 7, 2024

In this/my case would it mean that Firefox will redirect to the main long URL skipping the dynamic 'it' rule?

No, it would still match the rule

@github-project-automation github-project-automation bot moved this from Todo to Done in Shlink Mar 7, 2024
@acelaya
Copy link
Member

acelaya commented Mar 8, 2024

I have just released v4.0.1, which includes this fix.

For the docker image, you will need to wait for this job to finish https://github.com/shlinkio/shlink/actions/runs/8200381298

@morphinestyle
Copy link
Author

Version 4.0.2 with the language dynamic rule works a charm!
Thank you very much Alejandro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
2 participants