[4.0] Fix uninstalling extensions

[SharkyKZ]

Pull Request for Issue #24155.

Summary of Changes

Use correct event in Extension - Finder plugin. Only run when installation is successful.

Testing Instructions

Uninstall any extension.

Expected result

No errors.

Actual result

An error has occurred.
    0 Too few arguments to function PlgExtensionFinder::onExtensionBeforeUninstall(), 1 passed in /libraries/src/Plugin/CMSPlugin.php on line 287 and exactly 3 expected

Documentation Changes Required

No.

[infograf768]

@Hackwar @wilsonge
RANT:
It looks like this plugin only use is IF anyone adds in a language pack a txt file, which we already discussed as not possible to do via crowdin. It was apparently sneaked in through #21327 and merged without tests...

[Hackwar]

It was a mistake that this was part of that PR and I informed George about that right after he merged that. George is still pondering what to do. It was not my intention to "sneak" that in.

However, I'm astonished that you are so vehemently against this, since this solves a multi-language issue and you haven't provided an alternative solution to solve this so far. I'm not asking you to code a solution, but to propose a way that we could implement this. Believe me that I'm open for any proposal that works, but I don't see how this could be done in any way with crowdin, as the length of the list of common words differs from language to language.

[infograf768]

See discussion here:
#20391 (comment) and following

And I see you also have added the en-GB.com_finder.commonwords.txt file although we have explained you the issues with crowdin...

[brianteeman]

Surely you wouldn't translate the common words as they are unique for each language

[infograf768]

Evidently not.
the issue as we repeatedly said is:

1. We are not aware of any TT who would create such file. Therefore it would be the work of coders.
2. We think that such files should NOT be in the language packs, but as a sort of library. As we do for calendar locales, i.e. usable, when they exist.
   A unique fr file would be used for all fr-XX languages.
   A unique en file would be used for all en-XX languages.
   A unique de file for all de.XX

[Hackwar]

I accidentally pulled in the PR for the common words into the other PR. I did not try to sneak that in, as I said.

And again: give me a way to provide these lists with the language packs and I'm happy to do that. But so far you only told me no, without any possibility to fix this. I have no idea what crowdin can or can't do. Considering that we also have variable PHP code in the language packs, I find it really hard to believe that there is no way for us to add a static asset file.

[infograf768]

Read my lips:
#24163 (comment)

[infograf768]

These files should be available in
media/system/commonwords/en.commonwords.txt for example

And a new parameter could be added to the en-GB.xml(s) and others on the model of
<calendar>gregorian</calendar>

i.e.
<commonwords>en</commonwords>

[Hackwar]

It is not code. It is not a single file per language family.

[infograf768]

It is not code. It is not a single file per language family.

It should be as I explained above.
No more issues with packs, and availability for the few languages that "could" get one if coded...

[Bakual]

We already explained the limitations of our "official" translation tool.
The issue is exactly that this file isn't a translation. Crowdin can't handle that.
It also can't handle the localise.php file. We use an ugly hack so it works at least for some languages. But it's not nice and doesn't work reliable.
We should certainly not add more like this to the language packs.

The PR that accidentally was merged should be reverted.
Hannes can you do a PR to take that part out? As you said it wasn't intentionally in your PR.
Then we can look at other ways to implement it, without the use of language packs.

[Hackwar]

I can do a PR that removes this and provides another way to implement this, but I refuse to remove it for a to-be-decided solution that we don't know anything about yet. And no, it still isn't code. It is a plain list of common words in a language. There is nothing to code. I will not provide the lists for all the different languages, since I don't speak those languages and wouldn't know what I'm committing there.

[Bakual]

but I refuse to remove it for a to-be-decided solution that we don't know anything about yet.

I don't think extortion is a valid development strategy. Sorry that is not acceptable. It was merged by accident and has to be reverted, having a conditional on that revert is just wrong.
Nobody opposes your general idea and there were even ideas provided how it could be handled. We just said handling it in a language pack itself will not work due to not being able to manage it (practical reasons).

And yes, I'm aware it's not real code. But it's also not a translation. It's plain data. It's basically a textfile based database.

[wilsonge]

OK Right guys please chill. We don't need to fight about this. I'm just catching up on this from my holiday and have just pinged JM on glip and can confirm on the 2nd March Hannes did ping me about accidentally merging this but didn't have time to action anything with my preparation for Joomla Day France. So if there's anyone to blame it's me for not code reviewing it properly in the first place.

There's no reason to not merge this PR right now as it fixes the issue of uninstalling extensions (and I think it's going to be impossible the revert the original PR in an automated way anyhow given it was two PR's merged together).

I'll talk to @infograf768 tomorrow and come up with a way as to whether we can just keep what we have in core and use one of the alternative proposed methods - or simply revert as too much will need changing. But either way come up with a concrete plan of action. Because it's clear right now a patch along these lines is needed for smart search to work with non-english languages. Now for once can we take this offline - we're all senior contributors to the project - and not fight it out to the death in public. Thankyou