Updates Requested PR-5.3-dev Pending

User tests: Successful: Unsuccessful:

avatar tekvishal
tekvishal
22 Feb 2025

Pull Request for Issue # 44898

Summary of Changes

Made changes in this file components/com_tags/tmpl/tag/default_items.php
remove htmlencode for space i.e %20 and replaced with blank space

Testing Instructions

  1. Create a article
  2. add intro image, filename shoud consist of blank space.
  3. Add tag to the article.
  4. Goto menu -> add new menu item -> select type-> tags->Tagged items
  5. Create a tagged items menu item that show the list of articles with a specific tag
  6. Enable the options "Item Images -> Show"
  7. Goto Frontend -> menu -> select newly created menu

Actual result BEFORE applying this Pull Request

Image was not visible

Expected result AFTER applying this Pull Request

image should be visible

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
4.00

avatar tekvishal tekvishal - open - 22 Feb 2025
avatar tekvishal tekvishal - change - 22 Feb 2025
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Feb 2025
Category com_tags Front End
avatar tekvishal tekvishal - change - 22 Feb 2025
Labels Added: PR-5.3-dev
avatar crommie crommie - test_item - 22 Feb 2025 - Tested successfully
avatar crommie
crommie - comment - 22 Feb 2025

I have tested this item ✅ successfully on b6625f0

Tested on Joomla! 5.3-dev on the test server
Patch Applied using Joomla! Patch Tester

Followed instructions, works as described


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

avatar pe7er pe7er - change - 22 Feb 2025
Title
Issue#44898: Image not showed in tagged items list if the image has spaces in the name
[5.3] Image not showed in tagged items list if the image has spaces in the name
avatar pe7er pe7er - edited - 22 Feb 2025
avatar webfeuerflo
webfeuerflo - comment - 22 Feb 2025

successfully tested. I manually edited the file and now the image is shown

avatar dynamicsites
dynamicsites - comment - 22 Feb 2025

Issue fixed after patch.

avatar fgsw
fgsw - comment - 22 Feb 2025

@webfeuerflo @dynamicsites Can you please mark your test as successfully and submit the test result at the Issue Tracker so the test count.

avatar brianteeman
brianteeman - comment - 22 Feb 2025

This is not the correct fix. It should be using the layout

avatar webfeuerflo webfeuerflo - test_item - 22 Feb 2025 - Tested successfully
avatar webfeuerflo
webfeuerflo - comment - 22 Feb 2025

I have tested this item ✅ successfully on b6625f0


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

avatar dynamicsites dynamicsites - test_item - 22 Feb 2025 - Tested successfully
avatar dynamicsites
dynamicsites - comment - 22 Feb 2025

I have tested this item ✅ successfully on b6625f0


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

avatar richard67
richard67 - comment - 22 Feb 2025

This is not the correct fix. It should be using the layout

Not setting to RTC despite of successful tests for this reason.

@tekvishal Could you modify this PR or make a new one to do it the right way? That would be much appreciated.

avatar tekvishal
tekvishal - comment - 22 Feb 2025

Sure I'll update the PR

On Sat, 22 Feb, 2025, 6:54 pm Richard Fath, @.***>
wrote:

This is not the correct fix. It should be using the layout

Not setting to RTC despite of successful tests for this reason.

@tekvishal https://github.com/tekvishal Could you modify this PR or
make a new one to do it the right way? That would be much appreciated.


Reply to this email directly, view it on GitHub
#44960 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/BMKL6536XOMWAMKPIIDALHD2RB3ARAVCNFSM6AAAAABXU2Y5QCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNZWGIYTCMJYGQ
.
You are receiving this because you were mentioned.Message ID:
@.***>
[image: richard67]richard67 left a comment (#44960)
#44960 (comment)

This is not the correct fix. It should be using the layout

Not setting to RTC despite of successful tests for this reason.

@tekvishal https://github.com/tekvishal Could you modify this PR or
make a new one to do it the right way? That would be much appreciated.


Reply to this email directly, view it on GitHub
#44960 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/BMKL6536XOMWAMKPIIDALHD2RB3ARAVCNFSM6AAAAABXU2Y5QCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNZWGIYTCMJYGQ
.
You are receiving this because you were mentioned.Message ID:
@.***>

avatar tekvishal tekvishal - change - 22 Feb 2025
Labels Added: Updates Requested
avatar exlemor
exlemor - comment - 25 Feb 2025

(as mentioned in the Issue conversation -

Since this is not recommended for accessibility, SEO, and browser consistency reasons why are we even allowing it / entertaining allowing spaces in filenames? (I know I am going to get beaten up for that but figured I'd ask lol)

As mentioned by PixedBo, why don't we sanitize the file name...

It appears we have this:

use Joomla\CMS\Filesystem\File;
$safeFilename = File::makeSafe($originalFilename);

or update that function if need be.

(I realize I'm clueless in coding so I apologize in advance).

avatar tekvishal
tekvishal - comment - 26 Feb 2025

"This is not the correct fix. It should be using the layout"

Can you please help me understand what exact changes are required here?

avatar brianteeman
brianteeman - comment - 26 Feb 2025

I would expect this to use the layout
LayoutHelper::render('joomla.content.intro_image')

Add a Comment

Login with GitHub to post a comment