I don't really want to have a special case just for the type, but translate everything.

I don't really want to have a special case just for the type, but translate everything.

What do you mean by translating everything?

other values are from item data linked to their associated language.

I confirm that it works and fixes the error.

@cyrezdev Before I submit a successful test, perhaps you can quickly fix an identical problem in the admin panel on the Smart Search: Index page. If it can't enter this PR - let me know and I'll post a successful test.

I confirm that it works and fixes the error.

@cyrezdev Before I submit a successful test, perhaps you can quickly fix an identical problem in the admin panel on the Smart Search: Index page. If it can't enter this PR - let me know and I'll post a successful test.

I will see to fix it in another PR (and others things i've seen to be improved or fixed in com_finder...).

So better to test this PR alone ;-)

Please keep pull requests to single tasks.

I have tested this item ✅ successfully on f2c1581

I don't really want to have a special case just for the type, but translate everything.

What do you mean by translating everything?

other values are from item data linked to their associated language.

@Hackwar Could you check and report back more detailed if something is wrong or missing? Thanks in advance.

@Kostelano @viocassel Could you test again with the latest changes? Thanks in advance.

Thanks @richard67 for review! 👍

@Kostelano @viocassel Sorry, it needs test again, as i've changed the code with error fix, and removal of language hasKey control (as already processed in branchSingular).

Could you test new version?

Thank you!

I have tested this item ✅ successfully on f98078b

👍

I have tested this item ✅ successfully on f98078b

RTC

I have tested this item 🔴 unsuccessfully on f98078b

Hmm, I just tested and it doesn't work for me.

@Kostelano @viocassel Are you sure you have tested according to the description in issue #38846 with the latest changes in this PR?

@cyrezdev I think you need to do it in the way like you had it at the beginning, i.e. replace the

<?php echo Text::_($text); ?>

by

<?php $key = LanguageHelper::branchSingular($text); ?>
<?php echo $lang->hasKey($key) ? Text::_($key) : $text; ?>

in the <?php if ($type === 'Type') : ?>.

I will suggest a code change on GitHub.

Back to pending due to unsuccessful test.

@richard67 Yes, you're right, it's broken now. I tested late at night and somehow the latest changes to the assembly did not transfer. Sorry.

I can also confirm that your suggested change works.

Oh i'm sorry... Maybe too much tired due to covid... :-D

So, @richard67 not needed to control hasKey as already processed in LanguageHelper::branchSingular
But, this is what i missed in my last edit... 🙈

I've updated this PR, accordingly to my local code...

So, @richard67 not needed to control hasKey as already processed in LanguageHelper::branchSingular

Correct, I just saw that.

@Kostelano @viocassel the change was maybe not applied on your last test, so could you test again, and control code change is the same as the PR ?

Sorry for inconvenience, and thanks for your time testing! ;-)

I have tested this item ✅ successfully on 24b930a

Now it definitely works!

I have tested this item ✅ successfully on 24b930a

I don't really want to have a special case just for the type, but translate everything.

What do you mean by translating everything?

other values are from item data linked to their associated language.

@Hackwar Is your comment clarified with that response? If not, could you make any suggestions on what to change?

@SharkyKZ Still not ok with this PR after it has received some changes?

Did you not see #39353 (comment)? This fixes the issue for a specific hardcoded taxonomy but does not work for other taxonomies that may be translatable in the same manner.

Did you not see #39353 (comment)? This fixes the issue for a specific hardcoded taxonomy but does not work for other taxonomies that may be translatable in the same manner.

What you linked to is in another PR (for admin side): #39359

Not related to this PR (frontend search results)

What you linked to is in another PR (for admin side): #39359

It's probably a third party content type. For example, in this case, with such PR logic, smart search will work for the Contacts type (it really works - I checked it), but it may NOT work with a third-party component. The type may not be translated. Not sure.

What you linked to is in another PR (for admin side): #39359

It's probably a third party content type. For example, in this case, with such PR logic, smart search will work for the Contacts type (it really works - I checked it), but it may NOT work with a third-party component. The type may not be translated. Not sure.

I did this PR, as i'm working on a smart search plugin for one of my extension. ;-)
So, works the same (core and third-party).

@cyrezdev What Sharky and Hackwar mean is that they don't like this $type === 'Type' special treatment. Maybe a solution could be to just leave that if away and do the echo Text::_(LanguageHelper::branchSingular($text)); always. Not sure thought if that would be right. @Hackwar Was it that what you had in mind?

@cyrezdev What Sharky and Hackwar mean is that they don't like this $type === 'Type' special treatment. Maybe a solution could be to just leave that if away and do the echo Text::_(LanguageHelper::branchSingular($text)); always. Not sure thought if that would be right. @Hackwar Was it that what you had in mind?

Something like this:

                            <?php $text = LanguageHelper::branchSingular(implode(',', $taxonomy_text)); ?>
                            <?php echo Text::_($text); ?>

To explain why i did apply it only on Type:

• Type is more or less a system value (Article, Contact, Category). Not a data from $item object
• Others are data from $item object.

In don't have problems to apply it for all, but should we allow data translation ?

For example, if you have a category with title stored in database "Contact", this title will be translated.
Because of Language key "PLG_FINDER_QUERY_FILTER_BRANCH_S_CONTACT" in plg_finder_contacts.ini

Another example, you have an article with title "March".
If translation apply to all, you will have article title translated even if you don't want it (eg. in French, it will be translated in "Mars".
Because of Language key "MARCH" in joomla.ini

So, should we translate $item data?

Maybe another way without checking that branch name is "Type" is to check for branch parent_id.

What needs to be translated is all branches with parent_id = 1 (with ROOT as parent).

This could work:

                            <?php $text = implode(',', $taxonomy_text); ?>
                            <?php if ($branch->parent_id == 1) : ?>
                                <?php echo Text::_(LanguageHelper::branchSingular($text)); ?>
                            <?php else : ?>
                                <?php echo $text; ?>
                            <?php endif; ?>

What's your advice? @richard67 @Hackwar @SharkyKZ

Maybe another way without checking that branch name is "Type" is to check for branch parent_id.

What needs to be translated is all branches with parent_id = 1 (with ROOT as parent).

This could work:

                            <?php $text = implode(',', $taxonomy_text); ?>
                            <?php if ($branch->parent_id == 1) : ?>
                                <?php echo Text::_(LanguageHelper::branchSingular($text)); ?>
                            <?php else : ?>
                                <?php echo $text; ?>
                            <?php endif; ?>

What's your advice? @richard67 @Hackwar @SharkyKZ

That could work. 1 is the root item in the taxonomy table, so that should not change.

Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.

Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.

You mean level 1 ?

For me, ROOT has level 0 (zero).

Or you could check the „level“ column, that should be 2 when parent_id is 1 because that’s the first child level of the root item, and there should not be any other items with level 1, so all level 2 are children of the root item. Maybe that’s better, I don’t really know how safely we can rely on the root item always having id = 1. if someone deletes that with SQL and rebuilds the table, it might have another id than 1.

You mean level 1 ?

For me, ROOT has level 0 (zero).

Sorry, you are right. Root item has level 0, so its direkt children have level 1.

Sorry, you are right. Root item has level 0, so its direkt children have level 1.

In fact, it won't work... I've tested. All taxonomy types have level 1. (Article, Category, Contact...)

I have 2 proposals:

1. Using a different way to add taxonomy when we expect its value to be translated.

For example, instead of this currently in plg_finder_content:
$item->addTaxonomy('Type', 'Article');

We can set it like this:
$item->addTaxonomy('Type', '**Article**');

And then, in search/default_results.php, code like this:

                            <?php
                            $text       = implode(',', $taxonomy_text);
                            $branchName = trim($text, '**');
                            $key        = LanguageHelper::branchSingular($branchName);
                            ?>
                            <?php if ($text !== $branchName) : ?>
                                <?php echo Text::_($key); ?>
                            <?php else : ?>
                                <?php echo $text; ?>
                            <?php endif; ?>

It requires too an update for filter (select type) to trim **:
$key = LanguageHelper::branchPlural(trim($node->title, '**'));

And for content map filter in admin:
$key = LanguageHelper::branchSingular(trim($item->text, '**'));

This way, it is backward compatible (same behavior as before with no translation, if addTaxonomy not updated with ** to be translated).

2. No change with addTaxonomy and plugins required, but won't be 100% in control.
(example case of a category with title stored in database "Contact": #39353 (comment)).

WARNING !
A concrete exemple about potential issue of the solution 2:

• You have an article for the TV show "Friends".
• A 3rd party finder plugin have the constant PLG_FINDER_QUERY_FILTER_BRANCH_S_FRIENDS, the title of the article will be translated as it should not.

So code in search/default_results.php:

                            <?php
                            $text = implode(',', $taxonomy_text);
                            $key  = LanguageHelper::branchSingular($text);
                            ?>
                            <?php if ($text !== $key) : ?>
                                <?php echo Text::_($key); ?>
                            <?php else : ?>
                                <?php echo $text; ?>
                            <?php endif; ?>

SUM-UP:

• Solution 1 requires more changes, but it's B/C and it's i think a better way to have control on what could or not be translated.
• Solution 2 is simpler, but not reliable at 100%.
• other solution possible, but includes a refactory of com_finder, and to extend data table to manage translatable values. Not needed with solution 1.

Of course, if @richard67 @Hackwar @SharkyKZ and others agree to solution 1, i will update this PR accordingly. ;-)

@richard67 No news from other, but after some reflexions, i think the PR as it is, and was successfully tested, is valid.

As checking for type === 'Type'is not an issue, because changing "Type" value would be a B/C break for third-party integration, so it will stay "Type".

So, this PR to fix this issue with translation, works.

And for else (better handling of translatable or not value), it would need a refactory of com_finder, which could be set for J5.0
(and then, a $item->addTaxonomyType('article') would be better than $item->addTaxonomy('Type', 'article') i think.)

What's your opinion?

@cyrezdev I'm not an expert on the finder. Let's see what @Hackwar says.

@cyrezdev I'm not an expert on the finder. Let's see what @Hackwar says.

Ok, thank you!

But i don't think he has seen the possible issue of unwanted translation, if translating everything (not only Type value).

@Hackwar this PR fixes issue for com_finder in frontend as com_finder works currently, but it would need some refactory to extend its possibilities.

Possible issue in applying translation for all:

@richard67 @Kostelano @viocassel

As the previous PR got no news, and some requested translation for all values (#39353 (comment)), with no type check, i've redo a PR to simplify it, as i would expect this simple issue to be fixed asap! ;-)

So there the new PR: #39636

Thank you in advance for your testing! 👍