I have tested this item ✅ successfully on 6d69334

so you called

***Document->getHeadData();

to get the current (head) data

... which also includes (unfortunately ... ??)

$data['scriptText']  = \JText::getScriptStrings();

you customize / manipulate $data, and then you call ...

***Document->setHeadData($data);

but now you will be calling 2nd (or 3rd or 4th time if you call setHeadData multiple times)
JText::script on the same language strings
this is ok to do , but the problem is a little more complex ... (i think)

how do you know if the original call to JText::script($string, $jsSafe ...) was using false or was using true for $jsSafe, you don't know right ?

Also you don't know what the setting for $interpretBackSlashes was during the original calls

public static function script($string = null, $jsSafe = false, $interpretBackSlashes = true)

@ggppdk

this is ok to do , but the problem is a little more complex ... (i think)

I realized that as well but doesn't warrant incorrect use of a function. The real solution would be to keep track of which string wants to be jsSafe. A refactoring not to be done in this PR.

how do you know if the original call to JText::script($string, $jsSafe ...) was using false or was using true for $jsSafe, you don't know right ?

Nope, you do not know.

Also you don't know what the setting for $interpretBackSlashes was during the original calls

Yep, you don't know that either. There is more weird stuff going in here because the argument claims to be a boolean but in the function we check if it is an array :|

Also i see a logic / design problem here

$data['scriptText']
is supposed to have
$key => $already_manipulate_string

why the above ? , because that is what is given by

$data['scriptText']  = \JText::getScriptStrings();

you are not supposed to add or "re-add" them using ... Text::script aka remanipulating according to $jsSafe, and $interpretBackSlashes

instead you should some how
-- merge them with existing static::strings inside the Text class

$new_strings_array = array_merge(\JText::getScriptStrings(), $data['scriptText']);

// and then call ...
Factory::getDocument()->addScriptOptions('joomla.jtext', $new_strings_array, false);

• but there is no method in the API of Text class to do the above

also "fixing" this bogus behavior now may be considered a B/C break ...

@roland-d

i agree that this PR is better than what is happening now

i think making a full / proper fix is for J4

@roland-d

question,

why not call php \JText::getScriptStrings();
(to get already keys which already have been added via Text::script)

and then loop through $data['scriptText'] and only call Text::script only on new keys ?

@ggppdk

i think making a full / proper fix is for J4

I also believe that is the place to really refactor this.

and then loop through $data['scriptText'] and only call Text::script only on new keys ?

Not sure if this is enough because when I look at the original commit it was for cached data. Javascript translations are not available if caching is enabled. So from this I gather that when you load a cached page the JS has no language strings, so you need to run through all of them to be made available.

@roland-d

right,
missed this one about caching
but we can still do the check that i suggested and not re-add the keys already added ... to the static::$strings of Text class

Just , we need to guarantee that at least 1 call to Text:script is made inside setHeadData()
So after the loop we can do

Text::script('__DUMMY__');

that way you will get Text class to call

HTMLHelper::_('behavior.core');

Factory::getDocument()->addScriptOptions('joomla.jtext', static::$strings, false);

@ggppdk
Adding that DUMMY string doesn't sit right with me either as it feels like a workaround. Perhaps we should just focus on a better solution on Joomla 4.

Adding that DUMMY string doesn't sit right with me either as it feels like a workaround. Perhaps we should just focus on a better solution on Joomla 4.

i see,
i hope i contributed something here for a more complete fix in J4