Modal titles should be escaped properly

[dgrammatiko]

This should take care titles that contain quotes

Test

Make sure that nothing breaks

[roland-d]

@dgt41 Can't we just use this here:

JText::_('COM_BANNERS_BATCH_OPTIONS', true),

[dgrammatiko]

Yup, did that!

[infograf768]

This does not work here: it adds a slash before the single quote:
For example, when editing a single article menu item and using:
COM_CONTENT_CHANGE_ARTICLE="Sélectionner ou changer l'article"
The modal now loads OK but I get, for its title

[infograf768]

• @zero-24 is correct concerning user notes. The plural should not be taken off or we get:
  

[infograf768]

Found a not-so-elegant solution in the layout...
<h3><?php echo str_replace("\'", "'", $params['title']); ?></h3>

[Bakual]

That looks wrong. There must be a better way.

[infograf768]

That looks wrong. There must be a better way.

I hope so... that's why I wrote a "not-so-elegant" solution...

[Fedik]

just curious, what wrong with $this->escape($params['title']) ?

[infograf768]

that would work indeed

[infograf768]

@dgt41
The new much simpler change works fine!

[dgrammatiko]

Hi, some further infos here, the problem comes from this line:
https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/modal/main.php#L73
Following @infograf768 solution (reversed) solves the problem without the need to add all those true switches to each JText.
Also this is 100% B/C

[infograf768]

One tester more to get this merged

[Bakual]

Instead of using a custom preg_replace, please use the native function addslashes().
And then test also with double quotes " and backslashes \ as those will be escaped by this method as well.
I tested it locally and it worked fine.

[infograf768]

@Bakual
Agree with addslashes()

[dgrammatiko]

@Bakual @infograf768 done!

[infograf768]

OK here!

[roland-d]

All looking OK here. Setting to RTC as we have 2 successful tests.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7845.}

[ggppdk]

I think @Fedik 's suggestion
$this->escape()
which is:
htmlspecialchars(..., ENT_COMPAT, 'UTF-8');

is the prefered way to encode HTML Tag parameters:
so this should better
$iframeAttributes['name'] = htmlspecialchars($params['title'], ENT_COMPAT, 'UTF-8');

which will

• encode double quotes and other special characters,
• it will skip single quotes
• it will consider UTF-8 characters and not encode them

i mean it is used inside Joomla and extensions code extensively to encode HTML Tag parameters, why this is an exception ?

[dgrammatiko]

@ggppdk Georgeif you think that the current merged solution is inadequate please do a new pr!

[ggppdk]

i am not sure )) that is why my post was a question
i am often wrong too ))

i noted that in other places we are escaping the value of an HTML tag parameters in different way

[dgrammatiko]

The thing here is that the string has to be escaped for JavaScript not for html, so I'm not sure if your suggestion will work. Currently I'm not at my desk so I cannot test it....

[ggppdk]

addslashes will work for javascript
i am asking if it can break some UTF-8 characters making some words appear mispelled ?
from what i read if it is given a valid UTF-8 string, then the string will not break

[Bakual]

it will skip single quotes

The only issue was the single quote which needs to be escaped for JS. Otherwise JS thinks the string ends and the code is broken. So your suggestion would not work.

[ggppdk]

• you only need to escape (at least) double quotes to get parseable HTML
  <iframe name="text_with_escaped_double_quotes" ...>
• and for valid HTML (not just parsable) use on the HTML Tag parameter values
  htmlspecialchars(..., ENT_COMPAT, 'UTF-8');

but then i see that HTML created by the ...\modal.php

• is used in JS code that assumes that single quotes are escaped

SORRY i missed that !, so yes you are right, (as i said i am often wrong)

• single quotes need to be escaped (for current JS code to work)

just a note, i think it would be best that HTML creation in template is more consistent

1. inside templates files just create valid HTML
2. do escaping of single quotes at the place that PHP code is creating the JS code

• thus avoid workarounds like this, and also if modal.php is updated in the future, nothing will break
  e.g. if some adds a single quote anywhere, not just inside the HTML parameter values, but anywhere
• anyway current solution works and is B/C, no need to change something

@dgt41
thanks for your contribution, i am sorry i meant nothing wrong about your works

i wish i could contribute just i am involved in with 2 web softwares with too large code base
i need to update my own code and removed deprecated stuff

a reason for the comment, is that i remember my modals sometimes being broken (i don't remember why), and i ended up replacing them with jQuery modals