J3 Issue ?
avatar PhilETaylor
PhilETaylor
18 Oct 2018

Steps to reproduce the issue

install Joomla 3.9@9ffbb2a

Install @nikosdion Akeeba Backup using Install From Web interface

Expected result

Action logs translate everything a 3pd can throw at it

Actual result

screenshot 2018-10-18 at 15 54 51

  1. Two strings not translated

Also nice to have's

  1. Component Name is not Hyperlinked to the Extension page index.php?option=com_akeeba (It could be)
  2. Settings page of the component is not Hyperlinked to the settings page (It could be)
avatar PhilETaylor PhilETaylor - open - 18 Oct 2018
avatar joomla-cms-bot joomla-cms-bot - change - 18 Oct 2018
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 18 Oct 2018
avatar PhilETaylor
PhilETaylor - comment - 18 Oct 2018

Another extension on the first screen of the "Install from web" extension... JEvents as an example:

screenshot 2018-10-18 at 16 06 27

avatar mbabker
mbabker - comment - 18 Oct 2018

IIRC...

  • The language files for disabled extensions aren't being loaded
  • Not all of the extensions in those packages are enabled on a default install
avatar nikosdion
nikosdion - comment - 18 Oct 2018

Yes, the extensions with the broken language strings are not enabled by default. But this does not make it any less of a Joomla bug than it is.

I think it’s blatantly out of touch with the real world expecting developers to only ship enabled extensions by default. Plugins are for optional features. Otherwise we are regressing to the Mambo day’s where each developer implemented their own extensibility code and core hacks to make interesting things happen.

I guess I’ll just document yet another way Joomla is broken so I can at least point my users to it when they complain about yet another Joomla bug getting in the way of their experience...

avatar mbabker
mbabker - comment - 18 Oct 2018

I'm not saying it shouldn't be fixed, just shooting from the hip in trying to identify why things might be happening.

#22637 is also designed to help with some of the translation issues.

avatar PhilETaylor
PhilETaylor - comment - 18 Oct 2018

I was trying to find #22637 to tag @infograf768 here :-) as I know he has already worked on some issues like this. It cannot be left like this, as its a horrid user experience.

I can understand not loading language files for disabled things as being a good thing for performance on the frontend of a site, however for Action Logging these do need translating at the point of rendering this list. Users would expect that.

Note that Akeeba & JEvents - the two I tested with, are the first two installable on the frontpage of the Install From Web extension - I never singled these out, I just randomly acted like a user would and installed what was infront of me.

avatar mbabker
mbabker - comment - 18 Oct 2018

Another thing I just realized. ActionlogsHelper::loadTranslationFiles() can fail over on:

  • Loading language strings for a site component element
  • Loading language strings for an admin module
  • Loading language strings for site templates

(Look at the extension paths for all of those)

Edit: Extracted to new issue

avatar infograf768
infograf768 - comment - 19 Oct 2018

If this #22637 was merged, it would solve some of these issues... For example for Akeeba
User admin installed the plugin Quick Icon - Akeeba Backup Notification is corrected

But it does not only need an "approved" status, but a test OK on issues.joomla.org... or a merge on review, then we can add new PR for other issues.

But we can't solve obvious coding errors... Here for Akeeba
PLG_SYSTEM_BACKUPONUPDATE_TITLE
and
PLG_SYSTEM_AKEEBAUPDATECHECK_TITLE

for a simple reason: the path to these plugins language files is wrong because of this wrong <name>.
It should not include _TITLE
Once corrected in the xml, db and the ini files for these extensions and after editing the files concerned, I do get here the correct results.
here for PLG_SYSTEM_AKEEBAUPDATECHECK:

screen shot 2018-10-19 at 07 41 58

avatar infograf768
infograf768 - comment - 19 Oct 2018

I saw the same coding error for JEvents; using '_TITLE`...

avatar PhilETaylor
PhilETaylor - comment - 19 Oct 2018

I hate to say I told you so... There are going to be lots of these cases... are you really going to put the responsibility back on the 3pd to change their code? I cant see that going down well :) :) :)

avatar infograf768
infograf768 - comment - 19 Oct 2018

Actionlogs is all based on getting the manifest.

'name'           => (string) $manifest->name,
'extension_name' => (string) $manifest->name

If this does not correspond to the real name of the extension and its ini file, it will never work OK.
Yes, there is a responsibility of 3pds.
One gets what one codes and, honestly, core can't cope with these errors.

avatar nikosdion
nikosdion - comment - 19 Oct 2018

You are wrong. Have you read the official XML manifest documentation? Of course not. At no point does it say that translation keys need to follow a specific pattern. All it says is:

Note: The and tags are also translatable fields so that the name and description of the extension can be shown to the user in their native language.

Just because you decided to do something in the core in a certain way does not mean that it's law and everyone must obey.

Making an assumption about the language keys without reading them from the manifest is wrong and runs against the documented XML manifest format and the way the installer code works from Joomla! 1.5.5 through 3.8.13 inclusive.

avatar laoneo
laoneo - comment - 19 Oct 2018

@nikosdion please mind your tone. I'v removed part of your last comment.

avatar nikosdion
nikosdion - comment - 19 Oct 2018

I do NOT accept your remarks, especially since they imply that used unprofessional language (which I didn't). I called JM insulting, ignorant and incompetent. That was not a quip. I selected the adjectives carefully and I can explain the reasoning behind them.

When you tell me that following your documentation instructions to the letter, as validated by the behavior of your code in your product, is suddenly a bug in my code (after nearly 13 years of it working perfectly fine) then you are being:

  1. Insulting. You are accusing me of not doing my job right because... I followed your documentation instructions as validated by your code.
  2. Ignorant. Making that comment means that you don't know how your software works for the last 13 years. If you do know and you still make that comment then you're being malicious. From the little I have met him in person I don't believe Jean-Marie is malicious.
  3. Incompetent. Instead of exploring the root cause and fixing the bug in your software you shift blame. Passing the buck is incompetent leadership. Sorry, that's the opposite of competent leadership.

If you don't like the adjectives then please change your behavior.

Since neither of you exhibited any leadership skills, please allow me to help you understand what good leadership would have looked like.

It is a fact that XML manifests currently allow arbitrary keys for the name and description attributes. The core does follow a different convention but it's merely that; a convention not enforced by code. A step forward would be removing the name and description attributes from the manifest and using the core convention of []<extensionName)[_XML_DESCRIPTION] instead.

However, that would be a b/c break. The deprecation window closed with 3.8. So all we can do is deprecate in 4.0 and remove in 5.0 per the project's published semantic versioning.

However, this does not solve the immediate problem with the user action logs. We have two facts:

  • The current code uses the XML manifest language key but does not load the language file, which is a bug.
  • The proposed code (that JM linked to) does not use the XML manifest language key (instead constructs an arbitrary key of its own) but does load the language file. That's still a bug.

However, we see that out of 2 actions required, each solution only gets one wrong and it's a different one in either case. It would stand to reason to assume that we might be able to combine them into a correct solution, i.e. one which both uses the XML manifest key and loads the language file.

The competent leader would therefore have the following action items:

  1. Table a proposal to remove the name and description elements from the XML manifest and update the documentation to reflect the new convention.
  2. Examine the possibility of merging both solutions for our problem into one that works right.
  3. If the first item goes forward, revisit the code in the second item to add a failover to the implied default should the name element be missing.

I firmly believe that this is far more productive than telling 3PDs they are to blame for following your documentation instructions (if you don't understand why this is bad you may want to consider stepping down from your position) and then censoring them when they call you out on your behavior with adjectives describing accurately your behavior. More so when your last reply implies that unprofessional language was used in the redacted part.

You need to learn to lead instead of cast blame and censor.

avatar infograf768
infograf768 - comment - 19 Oct 2018

@nikosdion
My dear friend, I LOVE your adjectives.

FYI, I am not at the origin of the code, therefore

Insulting. I followed your documentation instructions as validated by your code.

I do not feel concerned by this. It's not my doc, nor my code. I only participated over the years.
I sincerely thought that the manifests had to be normalized —which indeed makes me an ignorant.
In no way I wanted to insult you or anyone here.

Ignorant. Making that comment means that you don't know how your software works for the last 13 years.

I totally accept to be called ignorant. The older I get the more ignorant I am. ;)
BTW, J is as much my software than yours.

Incompetent. Instead of exploring the root cause and fixing the bug in your software you shift blame. Passing the buck is incompetent leadership.

I am too incompetent to solve the root issue.
FYI, I am in no way in a leadership position.
But I agree with you: my competences are limited.

Note: I still have some Ouzo left when you come visit. ?

avatar nikosdion
nikosdion - comment - 19 Oct 2018

Next time I visit I'll bring tsipouro and make some meze (tidbits) to share ;)

avatar infograf768
infograf768 - comment - 19 Oct 2018

As long as that τσίπουρο is a kind of τσικουδιά (i.e., for all the ignorants around, does not contain any Pimpinella anisum) and is the result of a double distillation I will certainly appreciate it, in small quantity though. ;)
Without μεζέδες it would be like a bicycle without a saddle. Hurting.

avatar mbabker
mbabker - comment - 25 Oct 2018

Well, this is painful to work with. Just looking at logs from the two com_config contexts and com_installer...

  • App config (Global configuration) changes use the extension name com_config.application (why is an extension name even logged here!?)
  • Component config uses the com_foo naming convention, that's good to work with
  • The extension manager seems to log the already translated name (or the language key if that string hasn't been loaded)

The extension name should at least be consistently handled one way or another here...

====

As for loading the name from the manifest, this should work, but it's based on arbitrary convention and I think in 4.0 we finally need to cave in and force a naming convention on the extension manifest XML files (because if you dig into the installer code the current one is "first XML in directory with <extension> tag").

	/**
	 * Get the name of an extension from its manifest
	 *
	 * @param   string  $extension  Extension name
	 *
	 * @return  string
	 *
	 * @since   __DEPLOY_VERSION__
	 */
	public static function getExtensionNameFromManifest($extension)
	{
		static $manifests = array();

		$lowerExtension = strtolower($extension);

		if (isset($manifests[$lowerExtension]))
		{
			return $manifests[$lowerExtension]['name'];
		}

		switch (substr($lowerExtension, 0, 3))
		{
			case 'com':
				$extensionName = substr($lowerExtension, 4);

				if (file_exists(JPATH_ADMINISTRATOR . '/components/' . $lowerExtension . '/' . $extensionName . '.xml'))
				{
					$manifestFile = JPATH_ADMINISTRATOR . '/components/' . $lowerExtension . '/' . $extensionName . '.xml';
				}
				elseif (file_exists(JPATH_SITE . '/components/' . $lowerExtension . '/' . $extensionName . '.xml'))
				{
					$manifestFile = JPATH_SITE . '/components/' . $lowerExtension . '/' . $extensionName . '.xml';
				}
				else
				{
					// No manifest available
					return $extension;
				}

				break;

			case 'lib':
				$manifestFile = JPATH_MANIFESTS . '/libraries/' . substr($lowerExtension, 4) . '.xml';

				break;

			case 'mod':
				// We don't have information on which app is in use, so have site modules take priority over admin
				if (file_exists(JPATH_SITE . '/components/' . $lowerExtension . '/' . $extension . '.xml'))
				{
					$manifestFile = JPATH_SITE . '/components/' . $lowerExtension . '/' . $lowerExtension . '.xml';
				}
				elseif (file_exists(JPATH_ADMINISTRATOR . '/components/' . $lowerExtension . '/' . $lowerExtension . '.xml'))
				{
					$manifestFile = JPATH_ADMINISTRATOR . '/components/' . $lowerExtension . '/' . $lowerExtension . '.xml';
				}
				else
				{
					// No manifest available
					return $extension;
				}

				break;

			case 'plg':
				$parts = explode('_', $lowerExtension, 3);

				$manifestFile = JPATH_PLUGINS . '/' . $parts[1] . '/' . $parts[2] . '/' . $parts[2] . '.xml';

				break;

			case 'tpl':
				$extensionName = substr($lowerExtension, 4);

				// We don't have information on which app is in use, so have site templates take priority over admin
				if (file_exists(JPATH_SITE . '/templates/' . $extensionName . '/templateDetails.xml'))
				{
					$manifestFile = JPATH_SITE . '/templates/' . $extensionName . '/templateDetails.xml';
				}
				elseif (file_exists(JPATH_ADMINISTRATOR . '/templates/' . $extensionName . '/templateDetails.xml'))
				{
					$manifestFile = JPATH_ADMINISTRATOR . '/templates/' . $extensionName . '/templateDetails.xml';
				}
				else
				{
					// No manifest available
					return $extension;
				}

				break;

			default:
				// Unknown/Unsupported extension type
				return $extension;
		}

		if (!file_exists($manifestFile))
		{
			// No manifest available
			return $extension;
		}

		$manifest = JInstaller::parseXMLInstallFile($manifestFile);

		// If the manifest is invalid then stub it
		if ($manifest === false)
		{
			$manifest = array('name' => $extension);
		}

		$manifests[$lowerExtension] = $manifest;

		return $manifest['name'];
	}
avatar brianteeman brianteeman - change - 30 Oct 2018
Labels Added: J3 Issue
avatar brianteeman brianteeman - labeled - 30 Oct 2018
avatar PhilETaylor PhilETaylor - change - 10 Feb 2019
Status New Closed
Closed_Date 0000-00-00 00:00:00 2019-02-10 21:34:34
Closed_By PhilETaylor
avatar PhilETaylor PhilETaylor - close - 10 Feb 2019

Add a Comment

Login with GitHub to post a comment