- π«π·France nod_ Lille
One idea that works only with JS enabled is when submitting the form, adding disabled to all unchecked checkboxes, that will exclude them from the POSTed values. Not sure how well it'd work when unchecking a permissionβ¦
- π¬π§United Kingdom catch
I think #53 is worth looking into, we've already had this but for years with or without js and making it work js-only would be a big improvement over not at all.
- π«π·France nod_ Lille
I'm not sure we can help more on the frontend side, browsers only submit checked checkboxes already. So on my 4000 checkbox pages, there are only the 143 checked checkboxes sent in the POST request.
browsers may have gotten smarter about all this since then.
- πΊπΈUnited States tedfordgif
Aren't there "shadow" hidden inputs as well? In order to get rid of those you'd need to e.g. round-trip a hash of the current state to detect if another user saved permissions in the meantime. Then the front-end could submit only the checkbox transitions. That would require JS as well.
While we're talking JS, if we send the max_post_vars as a setting, we can stop the submission if we would exceed it.
Or turn it into a progressive enhancement exercise, where the no-JS form can only look at one role at a time, and only once you enable JS can you edit all roles at once.
- πΊπΈUnited States tedfordgif
Hmmm, another terrible idea: I wonder if you could implement the transition idea without JS using radio buttons. If previously checked, make the "checked" input have an empty value, and the "unchecked" input have a value attr that means "remove this permission from the role". You could even use CSS to get close to the current interface.
I'm guessing the radio value always gets submitted, though, even with a blank value.
And there are likely to be a11y concerns.
- π©πͺGermany Anybody Porta Westfalica
Reviving this as I just ran into the issue once again (and think it happened to clients > 30 times in the last years).
Maybe a good first shot would be to ensure a viable minimum for
max_input_vars
for Drupal installations when installing Drupal and in the status report as warning?Just having a mid-size Drupal project, for example the admin menu administration page (
/admin/structure/menu/manage/admin
) or the permissions administration typically grows > 1000 input elements very fast.
So before going to be much into details here, adding a hook_requirements() check formax_input_vars
being at least 10000, otherwise showing a warning (not an error) to raise attention would be super helpful I think.While discussing this for a long time, I think many people are hurt by having a too low default hosting provider value (typically '1000'). What do you think? Everything else as follow-up perhaps?
- π¬π§United Kingdom catch
@anybody while it only improves the permissions page, I think π Improve performance of the user.permissions.js script running in /admin/people/permissions. Needs review may have improved things there - do you have a site you could confirm that on?
- π©πͺGermany Anybody Porta Westfalica
Hi @catch and thanks for the fast reply! Maybe there's a misunderstanding. My comment is about forms with many inputs not being saved at all, getting people into trouble, not only on the permissions page, but also for example on the menu administration and other pages with many form elements.
It's not about performance, but a risky (mis)configuration in php.ini for Drupal.
- π¬π§United Kingdom catch
@anybody that issue at least theoretically also reduced the number of checkboxes that the permissions page submits, so it would be good to see what if any effect it has on a real site affected by this issue.
Agreed the permissions page isn't the only problem here though.
- π©πͺGermany geek-merlin Freiburg, Germany
This can and should be solved generically in the DB driver.
We can steal code from ye olde SQLite 999-limit issue: π Make SQLite faster by combining multiple inserts and updates in a single query Needs work
Note that there have been several approaches in that issue. The current approach that sends multple values via JSON is really elegant, but likely too SQLite specific. But a former approach had a parameter chunker, that likely every DB driver can use. See #2031261-86: Make SQLite faster by combining multiple inserts and updates in a single query β ff.
- π©πͺGermany Anybody Porta Westfalica
@geek-merlin: Are you sure this landed in the right issue?
It's database-related, yes, but what we see here can also happen without any database interaction, what ever is done with the form values. (Of course in most cases they will be stored in a database).
This issue is about forms with many inputs, so that the sent request exceeds
max_input_vars
and values are dropped without a warning.I'm not sure if this belongs into database system. Guess you maybe worked in several related issues? (That clearly exist)
- π«π·France nod_ Lille
The main issue is at the http/php level
@tedfordgif we have done all we can at the html level. no more cruft to remove. the shadow inputs are not submitted
- π©πͺGermany Anybody Porta Westfalica
@nod_ thanks! Could you maybe give a short comment on my suggestions in #60? That could be a quick win to at least introduce a warning if the limit is low?
Additionally, we could show a warning if a form submitted exactly matches
max_post_vars
and allow opt-out via settings.php? With a highmax_post_vars
value this warning would then become improbable.Once we have a plan, I think it would make sense to split this into sub-issues for the different points to implement. But it first needs a decision which way to follow.
Thanks!
- π«π·France nod_ Lille
hook_requirement make sense. i would prevent the form from being submitted with a warning if we know it will go over
- π©πͺGermany Anybody Porta Westfalica
@nod_ thanks! I don't think we can detect the form is going over, because we simply won't receive values for the values over the limit, but we should elaborate on that!
Let's add a first subtask then to implement a hook_requirements check for a
max_post_vars
limit < 10 000 to show on installation and in the status report. I'll ask our team.Here we should proceed checking how to detect a form submit that exceeds the limit.
- π«π·France nod_ Lille
we can get the max input var value and check with JS at submit for example. It's a pretty dangerous situation so special casing makes sense
- π¬π§United Kingdom catch
When max_input_vars is exceeded an E_WARNING is triggered according to https://www.php.net/manual/en/info.configuration.php#ini.max-input-vars - could we special-case this in the error handler and then do something with it?
- πͺπΈSpain tunic Madrid
Idea from #71 would be great. Using an arbitrary big number doesn't ensure issue is not going to happen.
Other option is adding the check in the Form API layer. During form rendering the number of variables needed can be known, and raise a warning to the watchdog. #71 idea is easier to implement, I guess.
- π©πͺGermany Anybody Porta Westfalica
Other option is adding the check in the Form API layer. During form rendering the number of variables needed can be known, and raise a warning to the watchdog. #71 idea is easier to implement, I guess.
Indeed, I think that would be better than JS and server-side. On the other hand, that could also get complicated with compound elements, but definitely solvable.
I hope I'll find the time in the next days to check #71 that would be best! Adding a warning to the status report for very low limit still makes sense to me anyway.