Skip to content

display max input vars#7453

Closed
brianteeman wants to merge 1 commit intojoomla:stagingfrom
brianteeman:maxvar
Closed

display max input vars#7453
brianteeman wants to merge 1 commit intojoomla:stagingfrom
brianteeman:maxvar

Conversation

@brianteeman
Copy link
Copy Markdown
Contributor

@brianteeman brianteeman commented Jul 17, 2015

Since php 5.3.9 there is a php setting for the number of max_inut_vars with a default value of 1000

This pr just adds a test to display the value in com_admin&view=sysinfo

Knowing the value is quite important now especially on sites with a large number of menus, usergroups or modules as if the value for max_input_vars is not big enough then saving will fail silently or will be truncated.

(Hopefully I will also be able to provide a plugin to test the value and see if it is high enough for the users site and display a warning message)

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Jul 17, 2015
@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Jul 17, 2015

@brianteeman

(Hopefully I will also be able to provide a plugin to test the value and see if it is high enough for the users site and display a warning message)

postinstall message?
Should we check this also on the installation? (maybe only as "warning")

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Jul 17, 2015

There are good security reasons for having the max_input_vars set at 1000
by default in PHP so you would not want to blindly increase it if there is
no need to do so.

The main case where it becomes an issue is if you have a total of more than
1000 modules, menu items and usergroups. So it is only something we should
be warning users about if they are close to the limit or have reached the
limit.

So no its not suitable for a pre-installation check or a postinstall message

On 17 July 2015 at 10:36, zero-24 notifications@github.com wrote:

@brianteeman https://github.com/brianteeman

(Hopefully I will also be able to provide a plugin to test the value and
see if it is high enough for the users site and display a warning message)

postinstall message?
Should we check this also on the installation? (maybe only as "warning")

—
Reply to this email directly or view it on GitHub
#7453 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Jul 17, 2015

Plugin coming soon as well.

malukenho
malukenho reviewed Jul 18, 2015
View reviewed changes
administrator/components/com_admin/views/sysinfo/tmpl/default_phpsettings.php
Copy link
Copy Markdown

@malukenho malukenho Jul 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix align

@widmann-it
Copy link
Copy Markdown

widmann-it commented Oct 24, 2015

Workend

@DavidTrenkle
Copy link
Copy Markdown

DavidTrenkle commented Oct 24, 2015

I have tested this item ✅ successfully on 965d632

works

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

@coolcat-creations
Copy link
Copy Markdown
Contributor

coolcat-creations commented Oct 24, 2015

I have tested this item ✅ successfully on 965d632

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

@dgrammatiko
Copy link
Copy Markdown
Contributor

dgrammatiko commented Oct 24, 2015

@brianteeman can you rebase this on latest staging, there is a conflict most probably form the file administrator/language/en-GB/en-GB.com_admin.ini

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Oct 24, 2015

I could if I knew how to but I don't. Each time I try it fails

On 24 October 2015 at 15:49, Dimitris Grammatiko notifications@github.com
wrote:

@brianteeman https://github.com/brianteeman can you rebase this on
latest staging, there is a conflict most probably form the file
administrator/language/en-GB/en-GB.com_admin.ini

—
Reply to this email directly or view it on GitHub
#7453 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Oct 24, 2015

@brianteeman please try

git pull origin maxvar
git pull upstream staging
git commit -am 'fix merge conflicts'
git push origin maxvar

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Oct 24, 2015

Strange that when it comes to some peoples PR the committer is happy
resolve merge conflicts

I;ve moved on. Happy to just close it

On 24 October 2015 at 15:58, zero-24 notifications@github.com wrote:

@brianteeman https://github.com/brianteeman please try

git pull origin maxvar
git pull upstream staging
git commit -am 'fix merge conflicts'
git push origin maxvar

—
Reply to this email directly or view it on GitHub
#7453 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Oct 24, 2015

Sorry I don't understand what you mean? Did it work or not?

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Oct 29, 2015

Closed

@brianteeman brianteeman deleted the maxvar branch October 29, 2015 00:30
@zero-24 zero-24 mentioned this pull request Nov 2, 2015
Merged
roland-d added a commit that referenced this pull request Nov 2, 2015
@roland-d
Merge pull request #8239 from zero-24/maxInputVars
e2e3a05 
Display max input vars (redo #7453)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Language Change This is for Translators

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@brianteeman @zero-24 @widmann-it @DavidTrenkle @coolcat-creations @dgrammatiko @malukenho @joomla-cms-bot