#Bug Smash Initiative

MR needs to be updated or a new one created based on #51. See the unresolved comments on the existing MR!

I can confirm this issue and I also ran into it on exactly ONE project, with no specific hosting settings or sth like that.

Some projects showed their update status correctly, others not. All very widely used contrib modules!

I first tried #55 with no success.
Then I tried the patch from #51, uninstalled and reinstalled the "update" module and finally it WORKS!

So RTBC+1 for #51 - should we make that a MR against 11.x?
We should also find a way to fix this without having to re-install updates module?

Sorry to reopen this but I find this wording very confusing: To add a translation, you must modify the configuration. Which configuration ? I had to check all kind of settings/configurations, in structure (content type, content type field), in settings (region and languages, content language and translation, configuration translation), in module settings (I faced this problem while translating meta settings with metatag module) etc., before I realize that I just had to change something in a field of the present form!

So I suggest to change this message to To add a translation, you must modify at least a field on this form.

I tried again on a fresh install of 10.2.6 and was not able to reproduce the problem. Since 10.2.6 is in security mode now and will not be receiving bug fixes, I am closing this as outdated.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue → starting from "Install Drupal core".

Thanks!

I've merged 11.x into this MR. Looks like the Hook Attributes work that got merged is triggering some deprecations that will need looking at.

Hi team, this ticket came up as the #bugsmash daily triage thread.

I'm just flagging that if we don't get an update on this within the next three months, it may be closed.

Cheers.

quietone → closed merge request !3529

Cross-referencing to fix.

Updating Current workaround, to match the default paths:

"drupal-scaffold": {
    "locations": {
        "web-root": "web/"
    },
    "file-mapping": {
        "[web-root]/sites/default/default.services.yml": false,
        "[web-root]/sites/default/default.settings.php": false
    }
},

#93 worked for me. Drupal version : 10.3.8

Merge request !7274 could not be applied.

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.

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.

Closing, no update for > 3 months.

Please reopen this if it is relevant in the future.

This still applies, however, core has been merged recently and I did that too instead of a rebase, with almost 60 commits, a rebase would be a ton of work. One option would be to squash it together into a single commit and force push that, I did that in a different issue.

I've tested the MR and it fixes the issue. Once MR comments are addressed it can go into RTBC.

In our case the whole content rendered as item in the feed is off not just the title.

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?

Just checking, will this not be committed to 11.0.x ?

Committed and pushed 35044e4519e to 11.x and 5b7419e8dfa to 11.1.x. Thanks!

FWIW we could backport this to 10.4.x / 10.5.x using https://www.drupal.org/node/3459876 →

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

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

Seems issue summary is good. Update the code against latest head.

nicoschi → changed the visibility of the branch 2546212-entity-viewform-mode-10.3.x to active.

@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.

hook_requirement make sense. i would prevent the form from being submitted with a warning if we know it will go over

@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 high max_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!

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

@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)

I have address the feedback, also verified with another method no type cast is added, also again re ran the test and its working as expected, pipeline is also green so moving this for NR.
Kindly review.

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.

Comment on MR.

Rebase seems good.

Only installing the Pathauto → module is removing this blocker and non-admin user can do the regular URL alias updates and creation. (But Core fix is recommended.)

Previously tagged for test which still appear to be needed.

Issue summary also appears to be incomplete.

smustgrave → credited larowlan → .

smustgrave → credited lendude → .

Rebase with many conflicts due to 📌 Convert entity type discovery to PHP attributes Needs review

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Merged and moved the new hooks to the new Hook class. Would be good if someone could double check that.

quietone → closed merge request !167

According to @larowlan, this is a problem in the formatting of a yml file. And there has been no discussion in 4 years to suggest that there is another problem. So, I am closing this.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue → starting from "Install Drupal core".

If this issue is resolved only by requiring external systems, I agree with @fgm to not mark this as duplicate.

The module shared by @codebymikey in the comment #29 implements some of the ideas from here, and can be augmented with other ones.

Listing some of the ideas gathered here so far for a quick recap:

- work within an early http.middleware service.
- ensure a consistent order for the query parameters.
- ignore query parameters based on an allow/disallow list.
- read this allow/disallow list set by the routing definition, gathered by a Routing listener service into KV records for quick checks.
- perform a single (1 per request?) preemptive (DB table size [better IMO] OR rows count) check before inserting an item into the cache, and (ignore until the next cron run OR prune) if full, instead of pruning AFTER the limit was exceeded.
- partial pruning using a popularity index, instead of the current max rows approach.

Maybe some of them have been addressed in the recent core releases.

MR !2690 don't work as expected.

Tested in Drupal 11.

1. Leaving one single item with - none shows error.
2. Selecting two different image formats will merge both at the same srcset and the type attribute gets removed. This makes fallbacks useless.

Workaround for second issue:
Make individual <source> items for every image format option.

Hacky workaround for image fallbacks using AVIF patch:
As Drupal 11 uses WebP by default, using 3202016 ✨ Let GDToolkit support AVIF image format Needs work allow us to use AVIF images, then we create a copy of every image style but using AVIF (convert). In the current theme make an alternative breakpoint group with copies for every fallback, for example, X-Large (WebP) and X-Large (AVIF), Medium (WebP) and Medium (AVIF), it's important to set a weight to avoid headaches. This way, every fallback will get its own <source> with type. We need to select a single style for every breakpoint to get this working.

material_base.inline_images.lg:
  label: 'Large'
  mediaQuery: 'all and (min-width: 1024px)'
  weight: 6
  mutipliers:
    - 1x
  group: material_base.inline_images
material_base.inline_images.lg_alt:
  label: 'Large (alt)'
  mediaQuery: 'all and (min-width: 1024px)'
  weight: 5
  mutipliers:
    - 1x
  group: material_base.inline_images

material_base.inline_images.md:
  label: 'Medium'
  mediaQuery: 'all and (min-width: 768px)'
  weight: 4
  mutipliers:
    - 1x
  group: material_base.inline_images
material_base.inline_images.md_alt:
  label: 'Medium (alt)'
  mediaQuery: 'all and (min-width: 768px)'
  weight: 3
  mutipliers:
    - 1x
  group: material_base.inline_images
...

Hello,

I created MR as part of new process of contribution.

Please review.

Thanks!

smustgrave → credited larowlan → .

Credited larowlan for triaging 🐛 Link Active

Tested on 10.3.7.

I confirm this does not happen with ckeditor5 from Drupal's core.

It happens with the contrib module ckeditor, the one created temporarily for the migration between cke4 and cke5.

My advice is then to enable the core ckeditor5 module, then switch all your text formats using the old editor to CkEditor5.

If you have errors when saving the text format, you may need to update your contrib modules which provide plugins for ckeditor.

Because of the major shift in 📌 Replace book_node_builder with entity fields Active I'm considering starting a 3.0.x branch

✨ Make book child content-type configurable per enabled book content-type Postponed I would like to land in 2.0.x though and think it's pretty safe if we land the update hook right.

IMHO if you're building a site now it's worth starting with v2 and get used to its (limited) changes, rather than having to deal with it later.

Hm. Good to know. We are trying to decide if we want to move from core to 1.x (which is a full release covered by the security policy, we also need to bring some patches with, but still fails some of our caching tests) or to 2.x (which we don't need the patches for, but might have more changes that need more QA). I'm unsure.

One is display the link in an anchor tag. Here the trim makes sense because you are not breaking the link.

I would argue that although the trim works within an anchor tag, it should definitely not be the default. On practically all sites I work on I have to modify this default (on each and every link field on each display mode). If we were implementing this new, there's no way we would choose to truncate by default (although it should still be an option)

Resolved most of the Test case issues, few pipeline issues are unable to fix.

Committed and pushed 68013115a83 to 11.x and c3a9311a660 to 11.1.x. Thanks!

> That route has the _csrf_token requirement. That gets removed by the array_diff in AccessManager::check because there is no request, which the csrf_token requirement needs.

We tried the patch and did just run into this as well.

On one hand, I think that can be improved in masquerade and is arguably even a bug. The controller still checks access, so it's not a security issue.. masquerade_target_user_access() or masquerade_switch_user_validate() really should be exposed as an access check and the route should not _only_ have the csrf access check.

But I'm also not sure that the approach here is correct. It's kind a neat to fall back to the url access, but probably also has some performance overhead, because it involves creating route matches and lookups and directly doing access checks on the entity really should be faster. And it's an implicit API change anyway because all implementations need to now always return their URL.

An alternative, more "direct" approach would be to pass around a cacheability object to getDefaultOperations (tricky to add with BC) and the hook (easy to add an extra argument) and require that implementations add their access cacheability to do that and let calls deal with that. Similar to how hook_tokens() works for example.