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

[4.3] Fix Media Field with Windows directory separator in value #39248

Merged

Conversation

joomdonation
Copy link
Contributor

@joomdonation joomdonation commented Nov 19, 2022

Pull Request for Issue #39203.

Summary of Changes

Sometime, on a window system, value stored for a media form field has this format images\headers\banner.jpg and it will throw warning and there will be notice like below when you edit the record contains that field:

Notice: Undefined offset: 0 in ...\libraries\src\Form\Field\MediaField.php on line 294

This PR just fixes that notice.

Testing Instructions

If you want to fully re-procedure the issue, you need to have Joomla setup on windows and follow instructions at #39203 (comment) to see the issue. However, there is a simple way to re-procedure it :

  1. Add the line of code below after this line https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Form/Field/MediaField.php#L263 . That will fake name of the image file in window directory separator format
$this->value = 'images\headers\banner.jpg';

Actual result BEFORE applying this Pull Request

You see notice like below:

Notice: Undefined offset: 0 in ...\libraries\src\Form\Field\MediaField.php on line 294

Expected result AFTER applying this Pull Request

No notice anymore.

@richard67 richard67 changed the title [4.3] Fix Media Field with window directory separate in value [4.3] Fix Media Field with window directory separator in value Nov 19, 2022
@richard67 richard67 changed the title [4.3] Fix Media Field with window directory separator in value [4.3] Fix Media Field with Windows directory separator in value Nov 19, 2022
@jwaisner
Copy link
Member

I have tested this item ✅ successfully on 546b4c4

Looks good. Tested on Windows 11.


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

@robbiejackson
Copy link

I've tested this and can confirm that it fixes the original issue 39203 which I raised.


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

@richard67
Copy link
Member

I've tested this and can confirm that it fixes the original issue 39203 which I raised.

@robbiejackson Please use the blue "Test this" button at the top left corner in the issue tracker, then select your test result and then submit, so your test is properly counted. Thanks in advance.

@robbiejackson
Copy link

I have tested this item ✅ successfully on 546b4c4

Tested using images uploaded from a front-end form and which resulted in the images being given pathnames which were of the Windows format (with backslashes). This included tests for wben the Media Global Config "Path to Images Folder" was set to a Windows-format pathname images\photos, as well as a linux format pathname, and the images uploaded were saved to that folder.

These image files were then treated as media form fields in the admin backend, and the tests confirmed that the fields displayed and were editable ok, both before and after the associated records (with the image pathnames) were saved to the database.


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 23, 2022
@laoneo laoneo merged commit 1e3f466 into joomla:4.3-dev Nov 25, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 25, 2022
@laoneo
Copy link
Member

laoneo commented Nov 25, 2022

Thank you!

@laoneo laoneo added this to the Joomla! 4.3.0 milestone Nov 25, 2022
@joomdonation joomdonation deleted the media_field_window_directory_separator branch November 25, 2022 10:32
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.

6 participants