PBF bug Small PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar machadoug
machadoug
28 Jun 2021

Summary of Changes

The Max Length was printing just the NUMBER as the attribute instead of maxlength="NUMBER"

Testing Instructions

See screenshot

Actual result BEFORE and AFTER applying this Pull Request

fixed-textarea-max-length-attribute.mp4

Documentation Changes Required

None

avatar machadoug machadoug - open - 28 Jun 2021
avatar machadoug machadoug - change - 28 Jun 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2021
Category Layout
avatar machadoug machadoug - change - 29 Jun 2021
Labels Added: ?
avatar Quy
Quy - comment - 29 Jun 2021

I am not able to reproduce this issue.

Created a custom text area field with maxlength value.
The markup has the maxlength attribute.

avatar machadoug
machadoug - comment - 29 Jun 2021

@Quy , according to the textarea field layout code this will only work if the $maxlength has the entire attribute instead of just an integer, for example, if the $maxlength = 'maxlength="1000"'

I just noticed there's no @var documentation, but IMHO it should work just like the other variables, which contain the value only instead of the attribute_name="attribute_value"

avatar Quy
Quy - comment - 29 Jun 2021

Your PR will break the custom textarea field. See markup.

<textarea name="jform[com_fields][text-area]" id="jform_com_fields_text_area"   class="form-control"           maxlength=" maxlength="1000""   ></textarea>
avatar brianteeman
brianteeman - comment - 1 Jul 2021

The character count requires two attributes in the xml

				maxlength="160"
				charcounter="true"
avatar Quy
Quy - comment - 5 Jul 2021

@machadoug You have the same issue with rows (4) and cols (20) attributes displaying only the values. It is not happening in core so it must be on your end. Please provide steps/code to reproduce.

avatar Fedik
Fedik - comment - 5 Jul 2021

@machadoug I suspect you use the textarea layout with your own field,
because for core textarea field the maxlength is set in the class:

$rows = $this->rows ? ' rows="' . $this->rows . '"' : '';
$maxlength = $this->maxlength ? ' maxlength="' . $this->maxlength . '"' : '';
$extraData = array(
'maxlength' => $maxlength,
'rows' => $rows,
'columns' => $columns,
'charcounter' => $this->charcounter
);

It looks strange to me, but it works like that since ~3.7 version

avatar machadoug
machadoug - comment - 5 Jul 2021

I was indeed using a custom Textarea field in my extension. I was under the impression that we should use only the attribute value, not attribute="value". Like the onchange, onclick and autocomplete attributes.

Maybe we should include something in the code documentation explaining it's a string and not an integer?

 * @var   string   $rows         rows attribute for the field.
 * @var   string   $cols         cols attribute for the field.
 * @var   string   $maxlength    maxlength attribute for the field.
avatar Fedik
Fedik - comment - 5 Jul 2021

I was under the impression that we should use only the attribute value, not attribute="value". Like the onchange, onclick and autocomplete attributes.

That is correct expectation.
But I cannot say why it like that for textarea.

Would be good to fix it. Only need to care about backward compatibility here.
The fix should move this

$columns = $this->columns ? ' cols="' . $this->columns . '"' : '';
$rows = $this->rows ? ' rows="' . $this->rows . '"' : '';
$maxlength = $this->maxlength ? ' maxlength="' . $this->maxlength . '"' : '';

to the layout.

And in layout doing something like:

$maxlengthAttr = '';

if ($maxlengt)
{
  $maxlengthAttr = is_numeric($maxlength) ? ' maxlength="' . $maxlength . '"' : $maxlength;
}

The same for rows and cols attribute.

avatar machadoug machadoug - change - 5 Jul 2021
Labels Added: Information Required
avatar Fedik Fedik - change - 10 Jul 2021
Labels Removed: Information Required
avatar joomla-cms-bot joomla-cms-bot - change - 10 Jul 2021
Category Layout Layout Libraries
avatar Fedik
Fedik - comment - 10 Jul 2021

@machadoug I have added that changes to your PR

It ready for testing:
Make sure textarea field still working as before.

07b2aab 10 Jul 2021 avatar cs
54c78ce 10 Jul 2021 avatar cs
avatar chmst chmst - change - 31 Jan 2022
Labels Added: ? ?
Removed: ?
avatar HLeithner
HLeithner - comment - 27 Jun 2022

This pull request has automatically rebased to 4.2-dev.

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

This pull requests has been automatically converted to the PSR-12 coding standard.

avatar Quy Quy - change - 27 Jul 2022
Labels Added: PBF ? ?
Removed: ? ?
avatar chmst chmst - change - 21 Oct 2022
Labels Added: ?
Removed: ?
avatar chmst
chmst - comment - 21 Oct 2022

I have tested this item successfully on 7b37d37


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

avatar chmst chmst - test_item - 21 Oct 2022 - Tested successfully
avatar Quy Quy - change - 24 Jan 2023
Labels Removed: ?
avatar Hackwar Hackwar - change - 21 Feb 2023
Labels Added: Conflicting Files
avatar Quy Quy - change - 24 Mar 2023
Labels Added: PR-4.3-dev ?
Removed: Conflicting Files ?
avatar HLeithner
HLeithner - comment - 30 Sep 2023

This pull request has been automatically rebased to 4.4-dev.

avatar MacJoom MacJoom - change - 30 Oct 2023
Labels Added: bug ? PR-4.4-dev
Removed: PR-4.3-dev ?
avatar chmst chmst - change - 24 Feb 2024
Labels Added: Small
Removed: ?
avatar HLeithner HLeithner - change - 24 Apr 2024
Title
Fixed missing maxlength attribute in textarea.php
[4.4] Fixed missing maxlength attribute in textarea.php
avatar HLeithner HLeithner - edited - 24 Apr 2024

Add a Comment

Login with GitHub to post a comment