? Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
9 Sep 2015

This should take care titles that contain quotes

Test

Make sure that nothing breaks

avatar dgt41 dgt41 - open - 9 Sep 2015
avatar dgt41 dgt41 - change - 9 Sep 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Sep 2015
Labels Added: ?
avatar infograf768
infograf768 - comment - 10 Sep 2015

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
screen shot 2015-09-10 at 08 16 06

avatar infograf768
infograf768 - comment - 10 Sep 2015
  • @zero-24 is correct concerning user notes. The plural should not be taken off or we get: screen shot 2015-09-10 at 08 23 20
avatar infograf768
infograf768 - comment - 10 Sep 2015

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

avatar Bakual
Bakual - comment - 10 Sep 2015

That looks wrong. There must be a better way.

avatar infograf768
infograf768 - comment - 10 Sep 2015

That looks wrong. There must be a better way.

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

avatar Fedik
Fedik - comment - 10 Sep 2015

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

avatar infograf768
infograf768 - comment - 10 Sep 2015

that would work indeed

avatar infograf768
infograf768 - comment - 10 Sep 2015

@dgt41
The new much simpler change works fine!

avatar dgt41
dgt41 - comment - 10 Sep 2015

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

avatar infograf768 infograf768 - test_item - 10 Sep 2015 - Tested successfully
avatar infograf768
infograf768 - comment - 10 Sep 2015

One tester more to get this merged

avatar Bakual
Bakual - comment - 10 Sep 2015

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.

avatar infograf768
infograf768 - comment - 11 Sep 2015

@Bakual
Agree with addslashes()

avatar dgt41
dgt41 - comment - 11 Sep 2015
avatar infograf768
infograf768 - comment - 12 Sep 2015

OK here!

avatar roland-d
roland-d - comment - 14 Sep 2015

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.

avatar roland-d roland-d - test_item - 14 Sep 2015 - Tested successfully
avatar roland-d roland-d - change - 14 Sep 2015
Status Pending Ready to Commit
avatar rdeutz rdeutz - change - 14 Sep 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-09-14 05:19:05
Closed_By rdeutz
avatar rdeutz rdeutz - close - 14 Sep 2015
avatar rdeutz rdeutz - reference | f1d0d71 - 14 Sep 15
avatar rdeutz rdeutz - merge - 14 Sep 2015
avatar rdeutz rdeutz - close - 14 Sep 2015
avatar Kubik-Rubik Kubik-Rubik - change - 14 Sep 2015
Milestone Added:
avatar dgt41 dgt41 - head_ref_deleted - 16 Sep 2015
avatar ggppdk
ggppdk - comment - 17 Sep 2015

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 ?

avatar dgt41
dgt41 - comment - 17 Sep 2015

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

avatar ggppdk
ggppdk - comment - 17 Sep 2015

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

avatar dgt41
dgt41 - comment - 17 Sep 2015

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....

avatar ggppdk
ggppdk - comment - 17 Sep 2015

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

avatar Bakual
Bakual - comment - 18 Sep 2015

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.

avatar ggppdk
ggppdk - comment - 18 Sep 2015
  • 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

avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone Added:
avatar zero-24 zero-24 - change - 28 Oct 2015
Milestone

Add a Comment

Login with GitHub to post a comment