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

The char/varchar textarea minimum height is incorrectly fixed to 7 rows #16935

Closed
setaou opened this issue Jun 1, 2021 · 6 comments
Closed
Assignees
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
Milestone

Comments

@setaou
Copy link

setaou commented Jun 1, 2021

Describe the bug

The char/varchar textarea minimum height is incorrectly fixed to 7 rows

To Reproduce

  1. Configure a small value for $cfg['CharTextareaRows'], like for instance 2 (which is supposed to be the default, according to the documentation)
  2. Open the form to edit or insert a record
  3. Observe that he textarea is 7 rows tall

Expected behavior

The textarea height is supposed to respect the $cfg['CharTextareaRows'] config option.

Additional context

The issue is located at https://github.com/phpmyadmin/phpmyadmin/blob/master/libraries/classes/InsertEdit.php#L584 and has been introduced by PR #14379.
Here we can see $textAreaRows = max($GLOBALS['cfg']['CharTextareaRows'], 7);, which actually sets the lower boundary of CharTextareaRows to 7.

@williamdes williamdes added Bug A problem or regression with an existing feature question Used when we need feedback from the submitter or when the issue is a question about PMA labels Jun 1, 2021
@williamdes
Copy link
Member

Bonjour @setaou
Could you post a before and after screenshot please ?

Thank you so much for the detailed report you made !

@williamdes williamdes added this to Needs triage in issues via automation Jun 1, 2021
@williamdes williamdes moved this from Needs triage to to be fixed soon in issues Jun 1, 2021
@williamdes williamdes added this to the 5.1.2 milestone Jun 1, 2021
@setaou
Copy link
Author

setaou commented Jun 1, 2021

Here with the pristine version of phpmyadmin and CharTextareaRows set to 2, you can see the textarea is 7 rows tall :
pma-before

Removing the max() at libraries/classes/InsertEdit.php line 584 makes it respect the requested size of 2 rows :
pma-fixed

@williamdes
Copy link
Member

Nice, I will have a look ;)

@sonipardeep
Copy link

I think, At libraries/classes/InsertEdit.php
It should be $textAreaRows = max(2, $GLOBALS['cfg']['CharTextareaRows']);

instead of
$textAreaRows = max($GLOBALS['cfg']['CharTextareaRows'], 7);

It is because max function returns maximum of two values so it will always return 7 and will not respect $cfg['CharTextareaRows'] = 2;

So we need to supply first argument 2 and second argument $cfg['CharTextareaRows'].

shucontech added a commit to shucontech/phpmyadmin that referenced this issue Jun 7, 2021
Signed-off-by: Saksham Gupta <shucontech@gmail.com>
@williamdes williamdes added has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete and removed question Used when we need feedback from the submitter or when the issue is a question about PMA labels Jun 19, 2021
@williamdes williamdes self-assigned this Jun 21, 2021
williamdes added a commit that referenced this issue Jun 21, 2021
Fixes: #16935
Pull-request: #16949
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jun 21, 2021
…tareaRows

Ref: #16949
Ref: #16935
Pull-request: #14379

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jun 21, 2021
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jun 21, 2021
Actually, one poped out on tests after #16935
Ref: #14936

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member

Hi @setaou
I just re-build the snapshot with the fix, could you test it ?

Link: (phpMyAdmin 5.1+snapshot)

@setaou
Copy link
Author

setaou commented Jun 22, 2021

Hi William,
The bug seems fixed in the snapshot.
Thanks for the good work !

issues automation moved this from to be fixed soon to Closed Jun 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
issues
  
Closed
Development

No branches or pull requests

3 participants