🇨🇦Canada @joelpittet

I'm cherry-picking pieces of this attack but one part (for me at least) was this 🐛 TypeError: Illegal offset type in isset or empty in FormValidator->performRequiredValidation() Needs review which I made a follow-up from this for

I realize this will need tests.

joelpittet → created an issue.

@timwood This latest solution seems to check all the boxes. I check if there is a base_path and strip it out only when the destination URL is starts with a starting / (which might be always), then the full URL built has a base_path included (can't seem to get around this without manually building it)

Adding another related issue #2418219: Deprecate destination URLs that don't include the base path → as I dig into this further. Specifically #2418219-22: Deprecate destination URLs that don't include the base path → from @claudiu.cristea

Or better keep the current behavior: if the destination starts with a slash, it has the base path prefix. No slash, it's the internal path or alias.

Bump, this would be nice to get in.

@Grevil I tried to do that one yesterday too but 2 problems:

1. Drush 13 (unlucky) wasn't installing on my Drupal 11 local environment
2. Looking at the new annotations for the class in core we are overriding I assume it's not going to be as easy as enabling it compatibility (but I need to just try it...), I saw some CRs that gave me pause though

Thanks @iler for jumping back to grant the extra access and for being a part of the community for the time you have.

By fits and spurts... I will close this but if you have any other RTBC's that I missed that you have been using for years (especially a bug fix) I can add you as a co-maintainer to keep things moving.

Closing this as outdated because the event subscriber was removed and replaced in 🐛 Suppress display of regular system menu blocks in Layout Builder RTBC

Missed this but yes this will require a 9.x only version as 8.x will not be compatible with this change

Thanks I have merged this in to the 2.x branch

There are big changes to plugin discovery and other things so we'll need to test this out before deciding if 1.x or 2.x is the right place for this.

Ran out of time to test and commit this so moving to the next release plan

Ran out of time to test and commit this so moving to the next release

I committed a few of the planned and RTBC'd issues. I will close this as the release happens and move to the 1.12 the remaining

joelpittet → created an issue.

Thank you @dougreen for applying my feedback. I have committed this for the next release.

Thank you and I appreciate the tests on this as well. I have committed this for an upcoming release of 1.11

Thanks all for your patience, I have committed this to the dev release for inclusion in the upcoming release.

Closing this as it was released a while ago.

There is not enough info and I don't know what twig tweak is intended to do, so best just try to do it and describe your experience (success or failure)

Going to close this as works as designed as per comment #2

This sounds like it would conflict with https://www.drupal.org/project/block_class →

I tried to reproduce the error you reported and could not on a stock drupal 10.2 site with the standard install profile, logged in and logged out with the user menu placed in the content area.

If you can provide more details to reproduce we can look at re-opening this issue.

This doesn't sound like critical, and it is hard to tell if it's a bug or a feature or a support request. I will leave it as a bug request for now and hopefully someone confirm the steps to reproduce

@jeroendegloire Before squashing this bug I wonder what got it into this case in the first place. Is it possible to put a breakpoint where this is and see what is happening with the pluginid in this case? Maybe this error is highlighting a bigger data problem in your site that should be addressed.

Merging the MR was proving challenging, made a small change and merged it manually. I manually tested it and works with layout builder and block layout and simplifies the code.

Also I reverted the code in 🐛 Duplicate menu blocks appear in Layout Builder choose block form Fixed as it does double the work as it was doing hook_plugin_filter_TYPE__CONSUMER_alter

I appreciate the intent behind this fix but the problem with coding standard fixes is that they disrupt the fixes in the queue for actual bugs and features and forces them to be re-rolled (unnecessarily).

I would recommend these fixes be fixed when the lines, themselves, are being fixed in active issues, or the very least the same files.

Thanks @BenStallings I have merged new gitlab CI template.

Thank you all I have committed the patch and pushed to the latest dev release. It matches \Drupal\system\Plugin\Block\SystemMenuBlock::blockForm fix to the same problem

Closing this as fixed again, please open a new issue if you are experiencing problems.

I may revert the commit here in favour of 🐛 Suppress display of regular system menu blocks in Layout Builder RTBC as it looks like it covers more ground than this does.

This is the issue I got added in, feel free to reach out to them through contact form #2615792: Offer to co-maintain → . I'm not sure who else has the permissions

I appreciate the intent behind this fix but the problem with coding standard fixes is that they disrupt the fixes in the queue for actual bugs and features and forces them to be re-rolled (unnecessarily).

I would recommend these fixes be fixed when the lines, themselves, are being fixed in active issues, or the very least the same files.

Thanks for everybody who looked into this and sorry for the delay in committing it. I've committed it for next release

While I appreciate everybody's effort, I am not confident in the motivation around this change though in the issue summary... .txt file readme's are valid as well. See this for reference:
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

I have no need/motivation to make this change so I will close. If there is an updated rationale around the need for such a change I might consider it in the future.

Thanks @jweowu for fixing the regression without too much changes. It seems like people are happy with this change and it will be in the next release

What issues are causing you most problems here? I see 9 RTBC issues 4 of which are bug reports that probably need to be addressed.

Oh darn, I don't have the right permissions to add you, jumped the gun there, I will just give you an RTBC

Welcome, I haven't used this in years so it's low on my priority list, I am glad to have you aboard Chris!

I'm seeing this as well, same sequence too

"SQLSTATE[HY000]: General error: 1366 Incorrect string value: '\\xC0\\xA7\\xC0\\xA2%2...' for column 'message' at row 1: INSERT INTO \"watchdog\" (

the error is happening when inserted as a message in watchdog for me.

And is triggered by an exposed form field tacked onto the "All" value:
?name=&order=field_person_lname&research_groups=All&research_interests=All%C0%A7%C0%A2%252527%252522%5C%27%5C%22&sort=desc

I have committed the changes, I'll see how that affects it all before releasing the changes.

Ok manually merged the MR because it wouldn't let me catch up the MR in the UI... thanks everybody for working on this and for the patch @anairamzap

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

I will close this as fixed. Thank you for the last patch @berliner and all others who were involved and care about IPE

I'm using the patch in comment #32, thanks @alexpott for posting it for 10.2

Probably need this to trigger the handler, cross referencing ✨ Implement Repair 301 Handler Active

This is still true ✨ Implement Repair 301 Handler Active just cross referencing as you need that before this will do anything, no?

Bumping to 2.0.x

I echo what @solideogloria said in #6 about the default branch but you can change it retroactively.

Anyways this looks great and is what I'd expect it to be doing, it's ready to be committed.

Retitled to better indicate the proposed solution's intent

@quadrexdev good to know there is duplicate, maybe a separate issue to replace that? I just created a separate MR for the missing 308 issue that might be a better place for that fix ✨ Status code 308 for Permanent Redirect Needs review

I am hoping with this change that we don't even validate it so we can allow non-standard codes to be ignored like LinkedIn's 999 for example.

Maybe since the redirect functionality is currently being worked on and not functional yet, this is enough to go in as-is?

There is likely more work to make it treated like 301, but this at least doesn't mark it as invalid.

First step is to not treat it as invalid I'd assume. I will start there. Moved to the 2.x branch

Can the scope just be "Provide 'Check links' button for running link checker"?

You could solve the other part with pathologic → or something like that?

In the latest patch @joseph.olstad there is a couple lines doing isset() followed by !empty() but !empty() does an implied isset(). Further the variable exists in all cases so you can just do if (!$var)

So

// for non-standard environment (docker & ubuntu) on the security issue
    if ((isset($uri['host']) && $uri['host'] == $basepath) && (isset($overridehosturl) && !empty($overridehosturl))) {
      if (isset($overridehostport) && !empty($overridehostport)) {
        $replacement = $overridehosturl . ':' . $overridehostport;
      }
      else {
        $replacement = $overridehosturl;
      }
      $host_path = $uri['scheme'] . '://' . $uri['host'];
      $url = str_replace($host_path, $replacement, $url);
    }

becomes

    // For non-standard environment (docker & ubuntu) on the security issue.
    if ((isset($uri['host']) && $uri['host'] == $basepath) && $overridehosturl) {
      $replacement = $overridehostport ? $overridehosturl . ':' . $overridehostport : $overridehosturl;
      $host_path = $uri['scheme'] . '://' . $uri['host'];
      $url = str_replace($host_path, $replacement, $url);
    }

Those variable names should be snake case too... $override_host_port

My preference would be to split the task up, easier for one to get accepted before the other.

It would be nice to have these 2 features separate but the second part is exactly what I need as the unpublished nodes are outdated content in a lot of cases.

Anyways RTBC #17 with the text changes.

MR has my proposed solution which is just check if it's numeric.

joelpittet → created an issue.

Thanks @Grimreaper, this allowed me to deploy after this change without a 500 error regarding that missing service.

Thanks for the patch @franknoel. This project could use a new release with this in it. I will patch locally for now.

Thanks @ramil g. Hoping this helps performance a lot because in D8+ there is a lot of entity loading via cache and this will help that out a ton!

Also, #9 is simple enough to fix the saving bug, but maybe the MR that @Eduardo Morales Alberti is working on might be a better fix in the long run...

Let's bump this to critical because this can mean data loss if you upgrade to 2.0.4 and consent is turned on with GA4 you will stop getting analytics.

It's quite confusing, mglaman (maintainer) suggests #3319523-8: Is Google Tag Manager using a new format for the container ID "g-xxxxx" instead of "GTM-xxxxx"? →

@mp I think I'm experiencing the same as you, though I am not sure it's the same problem as the OP because I reverted to 2.0.2 and it works, but 2.0.4 does not. Maybe you can try reverting to see if that helps you?

It is a PHP 8.1 First-class Callable Syntax change.
https://php.watch/versions/8.1/first-class-callable-syntax

Probably won't do this quite yet to leave Drupal 9 alone.

https://github.com/rectorphp/rector-symfony/blob/main/docs/rector_rules_...

That is what is causing this. Twig itself doesn't even do this: https://github.com/twigphp/Twig/blob/64ee590d469227705cb4bad179cbda90241...

#3428170-3: Automated Drupal 11 compatibility fixes for api → Same not sure about that TwigFilter change

I've patched dev with a new release shortly. Thanks for looking into it @solideogloria

@kostask Thanks for the note, that fixed it for me as well, I was missing something in my custom entity's annotation.

Actually I stripped the tags out from the label, this suggestion is no longer needed. Thanks for pointing this out @dunot.

I'm not exactly sure what fixed it but there was some major refactoring in 777c0f36dd5ecf2c57dbc739c3f8a1c14e59cc93 from bmcclure's fork so likely have them to thank.

I compared the forks and the biggest things I got were from bmcclure's fork. I did some clean-up to how the oid's are collected from EntityField fields.

Most of that was merged in 777c0f36dd5ecf2c57dbc739c3f8a1c14e59cc93

Thanks @bmcclure and @twistor and everybody else that was working on this for a while. Also sorry for the delay, I also didn't need it till now.

I replaced the escape with a strip_tags() as we have that in D7 as well and it will do the trick better.

Fixed in 777c0f36dd5ecf2c57dbc739c3f8a1c14e59cc93

Closing this as the change mentioned in #9 was committed in 🐛 List is incorrect if multiple values present per row Fixed

I am quite sure I fixed that in the commit from 🐛 List is incorrect if multiple values present per row Fixed let me know if that is incorrect

I think I just confirmed this with just the Tags field on Article in a fresh install.

I can't even save the view when making it "Required"

No valid values found on filter: Content: Tags.

Would you like to add that as a MR/patch @dunot?

Not worth the pain, closing as this became too much "deer in headlights" of inaction

This one only shows select field options that actually yield results.

https://www.drupal.org/project/views_dependent_filters → Will make filters visible or hidden depending on the other filters chosen. So like https://drupal.org/project/conditional_fields for Views exposed filters.

Hope that helps

This is interesting... we can maybe see what Drupal Core does for this in terms of options labels and values. We don't want to double escape things and also don't want to have an XSS attack vector

joelpittet → changed the visibility of the branch 3213298-values-with-ampersand to hidden.

This seems to be a duplicate of a patch I just committed 🐛 List is incorrect if multiple values present per row Fixed If that is not the case please let me know and I will try to reproduce the problem

Closing as a duplicate of 📌 Drupal 10 Readiness Fixed

This fixed an obvious bug, I cleaned up the coding style issues and committed it. Thanks @dunot, @DamienMcKenna, and @NWOM for working on this problem.

I'll compare the forks to see if I can merge some of the ideas back in but I've exposed the dev branch. Once I get past some of the obvious bugs I will put an alpha release out.

Closing this for now, if you have any MRs/Patches feel free to add them to 8.x-1.x branch

Thank you all for trying to get this ready and @loopy1492 for the MR and patch

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