🇨🇦Canada @joelpittet

Ok I think I got this solid, could use some eyes.

@alan ridgway Thanks for the report. A white screen usually means there's a PHP error being suppressed. Can you check your logs or enable error reporting locally and share the specific error message?

To enable error reporting, you can add the following to your settings.local.php:

$config['system.logging']['error_level'] = 'verbose';

Once enabled, try reproducing the issue again and let us know what error appears. That will help us pinpoint the cause.

Thanks for the write-up on the issue. Which hook are they using, does it provide the same functionality?

xjm → credited joelpittet → .

Echoing #97 claudiu.cristea — “I would say yes.” Drupal Core should ease the transition in 11.x and fully remove it in 12.x.

I don't grok what is suggested in #98 sorry @godotislate

@pdureau #95 can you point me to the technical debt you are speaking of, I was responsible for some of it, so I am curious what I broke in the future!

xjm → credited joelpittet → .

Let's close this as it's had it's day

I think coder dropped CSS recently, I will close this for stale clean-up

gitlab-ci is the successor, I don't believe this is relevant (cleaning up issues I was following)

gitlab-ci is the successor, I don't believe this is relevant (cleaning up issues I was following)

Is this issue trying to solve a similar problem?
🐛 TypeError: array_key_exists(): Argument #2 ($array) must be of type array, string given Active

Thanks for reporting this. Could you clarify the underlying problem you’re trying to solve?

The issue summary explains that the markup changes and that a fieldset is used instead of the core’s default form-element, but it’s not clear what the impact of that change is. For example:

1. Is the concern mainly visual consistency?
2. Does the current structure break theming or accessibility?
3. Are there usability issues with the new layout or behavior?

It would also help to explain how your proposed solution—using type="date" or datetime-local" while keeping the core markup—addresses that problem.

If possible, screenshots of the current vs. expected output or examples of where the new markup causes issues would make the case stronger.

Thanks!

Also note this issue may change the markup, depending on the outcome 🐛 Undefined array key "#type" in Drupal\Core\Form\FormHelper Active

@andreasderijcke re #8, can you elaborate this bug report in a new issue?

Closing this on #5's great response! Thanks @hubertinio

We will close this in a month without a response, otherwise just poking to clean up the queue.

The HTML5 date input doesn’t support partial dates like day/month without a year, so implementing this would be difficult without replacing the input type entirely. If you need this feature, it might be easier to fork the module and adjust the field type and query logic as needed.

Closing as Won’t fix.

Just changing the status to active as we don't have an MR yet (an cleaning up the queue)
https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →

Postponing this on 🐛 Undefined array key "#type" in Drupal\Core\Form\FormHelper Active please see if the latest MR resolves this issue for you.

Thanks a bunch @ramil g!

joelpittet → changed the visibility of the branch 3.0.x to hidden.

Thanks @albeorte, I wonder how you tested that, it's a class __construct change that will have a regression
see this issue 🐛 [regression] The new property \Drupal\Core\Form\ConfigFormBase::$typedConfigManager conflicts with some contrib modules Fixed

Given this Change Record https://www.drupal.org/node/3404140 → we have a few options:

1. Create a new branch 4.x for Drupal core >=10.2
2. Set the minor version on 3.1.x to require Drupal core >=10.2

Setting to needs work because the base class constructor is not called any more in that, and so the config manager is not being set.

Merged this one-liner

Actually I think this is safe enough to move forward with, discussed with Graber in Slack and while he may not use it, it will hopefully make it easier for others to contribute back.

I think the code that @ramil g added restores the labels—though it may change how they’re themed (markup and classes). It also looks like the #description gets silently dropped in the transition, so that’s something to watch for. Still, this is definitely a step forward!

Adding the js-form-wrapper class helps make #states work across all themes. Previously, the code unset the #type, and since not all #theme templates include js-form-wrapper or js-form-item classes, the state system didn’t know where to apply visibility logic.

Ideally, we’d find a #type that works out of the box so we don’t have to manually build up the render array. That would make it a proper "render element" again, which also helps ensure consistent behavior.

In any case, this isn't quite ready to be committed yet—but it’s moving in the right direction.

Thanks xjm, it was! 😄

Somewhat related to 🐛 Undefined array key "#type" in Drupal\Core\Form\FormHelper Active

Need a tester to see if it will work before committing.

Hmm maybe this status is stopping the merge...

Merging this as it's low stakes.

I approve this message ✌️

joelpittet → made their first commit to this issue’s fork.

Thanks @graber (chatted over at slack)

joelpittet → created an issue.

joelpittet → created an issue.

joelpittet → created an issue.

joelpittet → created an issue.

I think this is technically done.

We encountered this issue and investigated further. It turns out the problem is not limited to the Gin theme—although Gin does interfere with things somewhat. Even when using other admin themes, the #states visibility logic for the min and max input fields is not applied correctly.

Specifically, the data-drupal-states attribute is not being attached to the correct element. In the case of the Gin theme, it's being rendered on a wrapping <div> instead of the actual <input> elements for the min and max fields. And without the Gin theme, they don't have the data-drupal-states attribute at all!

As a result, the visibility behaviour breaks: instead of hiding just the inputs when the selected operator doesn't require them (e.g., not using "Between" or "Not Between"), the entire filter may hide unexpectedly (Gin theme), or the fields may appear when they shouldn't (all the other themes).

Just ran into this, thanks for putting this together, hopefully this can be merged soon.

joelpittet → made their first commit to this issue’s fork.

This caught me trying to upgrade to D11 and then got held back from Drupal 11.2. +1 for merge

@nathanraystone would it be possible you could roll those changes into the Merge Request instead of a patch? And do you think that change will make D11 incompatible with previous versions of Drupal? In that case maybe this should be moved to a 4.x

This partially works BTW (just used it again), the only thing I end up fixing is a bit of duplicates in config/optional and one function name hook_form_system_theme_settings_alter The test might have a few different use-cases but that is where the main issue when using it in the real world...

I was seeing this before applying the MR diff locally, tempted to mark as a bug report, but no idea how this impacts <=10.4. This resolves issues when using 10.5

TypeError: e.icons is undefined
o https://drupal.test/modules/contrib/ckeditor_iframe/js/build/iframeembed...
_createFormView https://drupal.test/modules/contrib/ckeditor_iframe/js/build/iframeembed...
init https://drupal.test/modules/contrib/ckeditor_iframe/js/build/iframeembed...
promise callback*./packages/ckeditor5-core/src/index.ts/init/m/< https://drupal.test/core/assets/vendor/ckeditor5/ckeditor5-dll/ckeditor5-dll.js?v=45.2.0:5
m https://drupal.test/core/assets/vendor/ckeditor5/ckeditor5-dll/ckeditor5...
init https://drupal.test/core/assets/vendor/ckeditor5/ckeditor5-dll/ckeditor5...
initPlugins https://drupal.test/core/assets/vendor/ckeditor5/ckeditor5-dll/ckeditor5...
create https://drupal.test/core/assets/vendor/ckeditor5/editor-classic/editor-c...
create https://drupal.test/core/assets/vendor/ckeditor5/editor-classic/editor-c...
attach https://drupal.test/core/modules/ckeditor5/js/ckeditor5.js?sy686v:373
editorAttach https://drupal.test/core/modules/editor/js/editor.js?v=10.5.0:304
attach https://drupal.test/core/modules/editor/js/editor.js?v=10.5.0:228
attach https://drupal.test/core/modules/editor/js/editor.js?v=10.5.0:211
attachBehaviors https://drupal.test/core/misc/drupal.js?v=10.5.0:166
attachBehaviors https://drupal.test/core/misc/drupal.js?v=10.5.0:162
https://drupal.test/core/misc/drupal.init.js?v=10.5.0:32
listener https://drupal.test/core/misc/drupal.init.js?v=10.5.0:20
domReady https://drupal.test/core/misc/drupal.init.js?v=10.5.0:26
https://drupal.test/core/misc/drupal.init.js?v=10.5.0:31
https://drupal.test/core/misc/drupal.init.js?v=10.5.0:34
ckeditor5.js:467:19

Thanks @alex.bukach!

This is so old, thanks for keeping the dream! My patches were for D7 so I'm hiding them.

Yes getTranslateRoute returns NULL but MenuLinkBase::isTranslatable() uses it and casts it to a boolean:

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

  public function isTranslatable() {
    return (bool) $this->getTranslateRoute();
  }

And, yes, in this case the following overrides that:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/menu_...

  public function isTranslatable() {
    return $this->getEntity()->isTranslatable();
  }

It's a bit confusing because the first is doing isTranslatable on the MenuLink, and the second is checking the entity referenced by the menu link. Doesn't that seem inherently wrong to anybody else?

In any case I am hopeful not to have the assignment inside a condition and second, if we are getting a route of the entity, we should check it, and not the menu link. I will post a proposed suggestion on the code.

Please create this an MR, the patch looks reasonable. What are the steps to reproduce this?

Thank you all for fixing that issue and making this work with Drupal 11!

joelpittet → created an issue.

joelpittet → changed the visibility of the branch 3500675-array-support-removed to hidden.

It looks like a lot of people might be interested in this feature. Could someone update the MR with the latest patch?

Also looking at the latest patch, it seems to be about language code, maybe this needs to be separated to a new issue, what problem are we solving here?

Oh while getting the issue ready to add this loop through fields... I see that the field will get created automatically on the first og menu that gets created...

Still needs fixing, but mitigated by finishing the config

Thanks for creating the issue @franceslui. When we looked at this together this afternoon, it appears this field is only used here, and nowhere else outside of tests. I believe, but could be wrong (will check with @claudiu.cristea), that the intent is for this to be field name agnostic, though a field is indeed expected.

My gut is saying use \Drupal\og\OgGroupAudienceHelper::getAllGroupAudienceFields:
https://git.drupalcode.org/project/og/blob/404bb45bd8849cca32f34e5a78c4f...
and get all the fields.

The only question this solution brings up is that this module should likely install a default as, og when group content is being created, or maybe it does and we deleted it, but from looking at the code it didn't seem to be the case...

The tests-only fail (as expected) by the way.

Thanks @Anybody for review, I put code suggestions that you can apply (might need push access to do so), but feel free to apply the suggestions or make your own in the comments you started.

Ok wrote a test, hope the local file I faked as external is appropriate in this case. Couldn't think of what to do for an "external" CSS file but it doesn't really need to be truely external.

https://git.drupalcode.org/project/drupal/-/commit/b957382#2fb629f0e45c5... This is where the JS part got committed for reference: 🐛 Stampedes and cold cache performance issues with css/js aggregation Fixed

Took a stab because I noticed in debugging JsCollectionOptimizerLazy had an exclusion that CssCollectionOptimizerLazy was missing.

Fixing that prevents this bug, I will see if I can write a test to prove it's fixed.

I’m digging into this because the error still appears in my logs month to month.

Here’s the asset that triggers the failure: core/lib/Drupal/Core/Asset/CssOptimizer.php:43

Array
(
    [type] => external
    [minified] => 1
    [weight] => -199.99963333333
    [group] => 100
    [data] => https://cdn.ubc.ca/clf/7.0.4-minimal/css/minimal-clf-full-7.0.4-bw.css
    [version] => 7.0.4
    [media] => all
    [preprocess] => 1
    [license] => Array
        (
            [name] => GPL-2.0-or-later
            [url] => https://www.drupal.org/licensing/faq
            [gpl-compatible] => 1
        )

)

This is coming off a request to:
/sites/default/files/css/css__xlnlyYvQDTExRxMzowYQQbU2Gkpxbvzgs_51zHi7ms.css?delta=1&include=eJxlj-EOwiAMhF-IySORwsqGFkpo56ZP79QZjfy7-y6X9iK3rKmKjYcwsCg7KGHmJvbXmACEZYRmP-KkM2Y0DaVykXRFp-AJXUyk2OyXDy9u5CaK2XoQNIEb2rIfBUp3NBMQBF3EBXG4qfXMKtqg9smz_Q8zisCE0gUTsQcyu6RULnZsSwU6Hfb9xMHgDFtXDxSHuA5-7ZJ9Iz4A2b18gw&language=en&theme=galactus_cs_ext

Which attempts to load a library from my theme:
galactus_cs_ext/clf-fw-bw
That library is no longer used. So this request seems to be coming from a cached, malicious, or stale reference, and causes the

“Only file CSS assets can be optimized”

error.

For reference, here’s how you can decode the include hash via the command line:
php -r "print_r(@gzuncompress(base64_decode(str_replace(['-', '_'], ['+', '/'], 'INCLUDE_HASH_GOES_HERE'))));" Or you can use UrlHelper::uncompressQueryParameter() in code.

I’m not sure how to categorize this type of bug—it stems from a library that used to be used, still exists, but is no longer active in the theme. Yet the request for it persists and causes an exception rather than a graceful fallback or warning.

You might see why I confused as to the "minified" as it looked like a type: "external" is being "minified", and is thrown in a if ($css_asset['type'] != 'file') {

The library definition is:

clf-fw-bw:
  version: 7.0.4
  css:
    base:
      https://cdn.ubc.ca/clf/7.0.4-minimal/css/minimal-clf-full-7.0.4-bw.css: { type: external, minified: true }
    theme:
      css/clf/clf.social-icons.css: {}
  dependencies:
    - galactus_cs_ext/cdn-clf-js

Our docs suggest to me that this looks right:
https://www.drupal.org/docs/develop/theming-drupal/adding-assets-css-js-... →

Ok, moving this back to RTBC. I share the concern about the constraint validation not working, but I think that’s a separate system that likely needs its own dedicated tests. This patch has its own tests, and the last thing we flagged — the assertion being "positive" — now looks correct to me. I also ran the constraint-only tests and confirmed they still fail as expected after the changes.

Thanks for cleaning this up, passing to Alex as he might understand it and it appears I made it for him. I am sure if it’s important it will come back. Closing to help out the process a bit further

Seems to do the trick, thanks @ramil g

joelpittet → created an issue.

@fernly, feel free to jump on slack if you have any questions. I am around on the #og channel though sometimes more available than others (like right now on a ferry)

Thanks for moving that over! Would you mind creating a merge request (MR) for it? That way the tests can run, as patches no longer trigger DrupalCI now that things are moving to GitLabCI.

Unless there’s a specific reason, I’d prefer to see this against the 2.x branch — though I understand if you haven’t upgraded yet.

Of course this will likely need a test to prove that we are solving a problem.

Thanks for moving that over, it might also be a duplicate? 📌 OG Access port Active

I should make this more prominent on the homepage or something:
https://www.drupal.org/project/og_access →

joelpittet → created an issue.

joelpittet → changed the visibility of the branch 2.1.x to hidden.

It probably should be an either/or scenario. I’d hazard a guess that Monolog takes over a bit earlier in the logging chain (though I could be wrong). In my case, I had both enabled, and Monolog was beating EmailLog to the punch. Disabling Monolog got EmailLog working as expected.

Just chiming in as a user (not the maintainer) — I think this might be “working as designed,” even if the design isn’t ideal. Since the actual functionality and configuration live in the submodules, enabling only the main module gives the impression something should happen, but it doesn’t. That could lead to some confusion or a false positive, but technically it’s behaving as intended.

@poker10 I think I got the unrelated changes out of the MR. Also I agree removing the hook would be a BC break for people who may have fixed their noisy logs with it, so leaving it would be my preference.

joelpittet → made their first commit to this issue’s fork.

Setting to Needs review to check out the merge request

@gngn great! Consider changing the status to "Reviewed & Tested by Community" to get this committed!

Need a second pair of eyes to confirm it does what it needs to before committing. So setting to Needs Review.

@herved I have committed it to 1.x too. Sorry I rushed and should have created a separate MR and just cherry-picked the 2 commits, a bit messy.

Thanks @loze, closing as outdated

crediting

Added a kernel test that exercises the new logic using node entities. I picked nodes because they’re the most common OG group type and we already have entity_test coverage elsewhere, so this gives us a bit of variety.
Happy to swap it to entity_test if you’d prefer uniformity—just let me know.
Test bot is green 🤖✅!

Moving to 2.x because that is where we are doing most of the dev. I feel like I would have run into and fixed this already, are the steps to recreate this accurate? I just created a webform without seeing the error in the issue summary.

The bigger question is because webform's aren't bundleable nor content entities, should they even see the options? I am not sure here, maybe @claudiu.cristea or @amitaibu can chim in?

Just a heads-up: assigning tasks to yourself can be a bit of a double-edged sword. It’s easy to lose track of them, and they can end up stalled. When in doubt, it’s usually better to leave them unassigned.

I am committing this to unblock the other tests, but please have a look at the tests or the views relationships if I made a mistake, though it seems to be doing the trick.

Thanks @alorenc I agree it's missing, I have merged that in to the dev branch.