I think the issue comes from the fact that we didn't define a default value in the table for the client_id. But your patch will work as well.

I was wondering if we could remove the folder as well. It doesn't give an issue in postgres however it seems unnecessary.

test: patch works as expected!

@infograf768 any opinions?

Note testers need to test with staging rather than 3.3.6 so that another unrelated fix for postgres is applied.

Tested the patch on mysqli:
Install new pack is fine
Update pack is fine too.
Can't test on postgres here.
@wilsonge What do you mean by "I was wondering if we could remove the folder as well."

I just meant about the 'folder' => ''

Doesn't matter. Let's just get this merged!

RTC

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4923.

You can see from line 378 what it should have been, for those interested in doing the right thing!

The whole file is weird imho.
It contains some older 1.5 stuff (see the pdf fonts)
At the time this file was created (2009), we were still installing languages by folder and not as packs.

Looking at the _update table, one will see we now only use packs and not language zips per client.
The client is always 0 in the table and the folder is empty.
As you may know we forbid uninstalling languages, we only allow uninstalling language packs.

Technically I guess some one "could" install a site only or admin only language with some update server and that the client would be necessary. This confusion needs indeed clarification.

I did see 378 actually. Both lines were introduced at the same time as this both containing different codes (yes I did track this back before the PR to check because I saw that line) joomla/joomla-platform@78828a1#diff-4adc3c570aaac147ab74a2ad37f06b64R221

For that reason and not knowing what the original intention my opinion was that we should keep things as close together as possible.

If we want to do a larger refactoring then it should be done in a separate PR

Suggest to have install and update at least the same.
Surprised to see the update removed upon installation despite the type being package instead of language.
Line 378 proves the folder criteria is redundant. Suspect the client_id to be redundant as well.
Also expect the version to be checked, both on install and update. Updates should only be removed when the same or more recent version is installed.

Agree that this also should be done for file and component, in separate PR's. Don't see the need for folder, only used for plugins. Don't see a need to check on client_id if not appropriate.