? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
30 Dec 2021

Summary of Changes

PR #28549 added a force ltr character before the version number is displayed to prevent issues when using an rtl language.

It should not have been added to this instance as it is only used for the update log file which is always in english and therefore LTR and the special character is not recognised in plain text

Testing Instructions

code review as its an obvious fix

Actual result BEFORE applying this Pull Request

the logs\joomla_update.php file will contain a line similar to
2021-12-30T08:12:30+00:00 INFO ::1 update Update started by user Brian Teeman (608). Old version is ‎4.0.6-dev.

Expected result AFTER applying this Pull Request

the logs\joomla_update.php file will contain a line similar to
2021-12-30T08:12:30+00:00 INFO ::1 update Update started by user Brian Teeman (608). Old version is 4.0.6-dev.

Documentation Changes Required

none

644708f 30 Dec 2021 avatar brianteeman [4.0
avatar brianteeman brianteeman - open - 30 Dec 2021
avatar brianteeman brianteeman - change - 30 Dec 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Dec 2021
Category Administration com_joomlaupdate
avatar richard67
richard67 - comment - 30 Dec 2021

I agree that no HTML markup (and the unicode LTR mark can be considered as such) shall appear in a plain text log file.

Unfortunately the log is a PHP file, so some people might have some extension which shows it in the backend and so in a browser. Maybe this is how it came that the LTR mark was added.

I think it was wrong from the beginning on to use a PHP file as log file. It should be plain text with extension "log" or "txt".

If using PHP, the content should me marked somehow as text only, or it should produce HTML markup (with which we then could use the LTR marks or better ways again to show the version with LTR direction in the right way).

We should consider to change that for 5.0.

avatar richard67
richard67 - comment - 30 Dec 2021

I have tested this item successfully on 644708f

Code review.


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

avatar richard67 richard67 - test_item - 30 Dec 2021 - Tested successfully
avatar richard67
richard67 - comment - 30 Dec 2021

@brianteeman Regarding the RTL display of version numbers at other places than the log (for which I had started to work on the 3.10-dev branch as you maybe remember): If there are some issues remaining after your CSS cleanups, I can make a PR to fix them. I have already prepared most of it. In opposite to my previous attempt I will not use a helper but it will work the same way, wrap it into a span with dir=ltr if necessary. But I think it has been wrong for so long time and we had not many issues with it, so I think there is no need to hurry. Let me know if you disagree.

avatar PhilETaylor
PhilETaylor - comment - 30 Dec 2021

It should be plain text with extension "log" or "txt"

it’s a PHP file for security - when accessed through a direct url it doesn’t render the content but dies. (Or should do)

a txt or log file in the webspace would be publicly available to view which could be bad.

avatar richard67
richard67 - comment - 30 Dec 2021

It should be plain text with extension "log" or "txt"

it’s a PHP file for security - when accessed through a direct url it doesn’t render the content but dies. (Or should do)

a txt or log file in the webspace would be publicly available to view which could be bad.

@PhilETaylor I know. But that could be solved differently or the PHP log could use HTML markup or something like that.

avatar brianteeman
brianteeman - comment - 30 Dec 2021

changing the file type is beyond the scope of this PR

avatar richard67
richard67 - comment - 30 Dec 2021

changing the file type is beyond the scope of this PR

@brianteeman Yes, I know, that's why I have tested the PR with success.

avatar alikon
alikon - comment - 30 Dec 2021

I have tested this item successfully on 644708f


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

avatar alikon alikon - test_item - 30 Dec 2021 - Tested successfully
avatar alikon alikon - change - 30 Dec 2021
Status Pending Ready to Commit
avatar alikon
alikon - comment - 30 Dec 2021

RTC


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

avatar alikon
alikon - comment - 30 Dec 2021

RTC


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

avatar wilsonge wilsonge - change - 30 Dec 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-12-30 12:37:12
Closed_By wilsonge
Labels Added: ? ?
avatar wilsonge wilsonge - close - 30 Dec 2021
avatar wilsonge wilsonge - merge - 30 Dec 2021
avatar wilsonge
wilsonge - comment - 30 Dec 2021

Thanks!

Add a Comment

Login with GitHub to post a comment