Skip to content

Conversation

@phproberto
Copy link
Contributor

This solves an issue with installation showing "undefined" alert messages reported in #5331. Issue was introduced by me in #5257

Description

Basically the installation process is not adding alert message type translations properly (Before using any message type you have to add the JS translation with JText::script('MESSAGE'); , JText::script('ERROR');, etc.). If not the message is not really translated. This is discovered after removing the jQuery dependency in #5257

Basically what I'm doing now is testing that the translation of the message type has been defined and if not we skip adding the alert heading. This preserves exactly the previous behavior. Of course it would be better to add that message types translations to the installer but I think this fix makes sense anyway if third part extension developers forget to add the translation for example.

Backward compatibility

This PR has no B/C issues. In fact it fixes one.

Test

On the first Joomla installation screen of Joomla do not fill any fields and press the Next button. You should see the undefined alert message titles. After applyting the patch no titles should be visible.

@wilsonge
Copy link
Contributor

wilsonge commented Dec 9, 2014

Would it not be better to keep behaviour the same as JApplicationCms::enqueueMessage() and add a default of a message type if none given?

@dgrammatiko
Copy link
Contributor

I also prefer to fix this in the server side, 👍 from me @wilsonge

@waader
Copy link
Contributor

waader commented Dec 9, 2014

@test works!

Resolves the issue with the undefined message in #5331.

@dgrammatiko
Copy link
Contributor

@test success against #5099
before:
screen shot 2014-12-09 at 5 42 24

After:
screen shot 2014-12-09 at 5 43 18

@infograf768 infograf768 added this to the Joomla! 3.4.0 milestone Dec 10, 2014
@infograf768
Copy link
Member

Works fine. Merging.

infograf768 added a commit that referenced this pull request Dec 10, 2014
@infograf768 infograf768 merged commit e0730fe into joomla:staging Dec 10, 2014
@phproberto
Copy link
Contributor Author

@wilsonge as I said this fix is not incompatible with anything we want to do in the server side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants