Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert any special characters from 'Site Name' settled in Global Con… #17209

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

nppsbh
Copy link
Contributor

@nppsbh nppsbh commented Jul 21, 2017

…figuration

Example
Site name: my "website name"

HTML output
without this change:
<img src="/administrator/templates/isis/images/joomla.png" alt="my " website="" name""="">

with this change:
my "website name"

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

…figuration

Example
Site name: my "website name"

HTML output
without this change:
<img src="/administrator/templates/isis/images/joomla.png" alt="my " website="" name""="">

with this change:
<img src="/administrator/templates/isis/images/joomla.png" alt="my &quot;website name&quot;">
@brianteeman
Copy link
Contributor

Not checked this but surely if it correct them the change has to be made everywhere that we get the sitemap!e and not just in the login

@infograf768
Copy link
Member

The change is correct when the site name is used in html.
Beware when $sitename is used for mails. In that case htmlspecialchars should not be used.

@Heggi93
Copy link

Heggi93 commented Aug 22, 2017

Please add some detailed test instructions


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17209.

@brianteeman
Copy link
Contributor

@Heggi93 pretty much as it says in the original post.

In global configuration create a site name with a special character eg My "site" Rocks

On the administrator login page view the source code for the joomla logo. Before this PR the alt text will be invalid markup. After this PR the alt text should be valid markup.

Note that if correct then this change needs to be used in other places that we use the $sitename variable in a similar way

@ghost
Copy link

ghost commented Aug 26, 2017

I have tested this item ✅ successfully on d6c89e6

Source Code without PR:

bildschirmfoto 2017-08-26 um 08 03 34

Source Code with PR:

bildschirmfoto 2017-08-26 um 08 04 18
@brianteeman Thanks for Test Instructions.
@Heggi93 can you please test?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17209.

@sanderpotjer
Copy link
Member

I have tested this item ✅ successfully on d6c89e6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17209.

@joomla-cms-bot joomla-cms-bot added PR-staging RTC This Pull Request is Ready To Commit and removed PR-staging labels Sep 1, 2017
@ghost
Copy link

ghost commented Sep 1, 2017

RTC after two successful tests.

@mbabker mbabker added this to the Joomla 3.8.0 milestone Sep 1, 2017
@mbabker mbabker merged commit 3d2d25b into joomla:staging Sep 1, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 1, 2017
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.

7 participants