mstrelan → credited jcnventura → .
@joseph.olstad "Needs work" requires existing work to be improved.. If we tagged everything which requires some future effort as "Needs work" you'd certainly agree that 99% of all issues should have that status. If this is not to be left as "Active", then the best status is probably "Postponed" until the point someone can come up with a solution.
Personally, as I said in #3, the upgrade path will never exist, but will instead be a document page, with the major points being what @hatuhay highlights in #14.
Another issue is just how "US centric" this page is. Of the 4 case studies presented, we have:
- UNICEF: international organization, with HQ in New York, US (fully support having this one)
- State of Georgia: the government of a US state
- Princeton University: a US university
- Planned Parenthood: a US NGO
We have many good examples of significant non-US sites, such as the European Union, paralympic.org who organize the Paralympic Games and Amnesty International to name a few good examples from Europe, and I'm pretty sure we can find one good example from each of the other continents (South America, Africa, Asia and Oceania).
Definitively cut down on the "1 in 8 websites". I don't think it was ever true, but it's just getting less true each day. Maybe add a little (*) to the word websites to mean "as defined by BuiltWith's Top 10K list".
My problem is that this is such a blatant falsehood that anyone coming to Drupal for the first time would lose trust in us when they try to confirm this extraordinary claim, and then fail to do so.
At the moment, this value should be:
- BuiltWith: 1 in 40 according to https://trends.builtwith.com/cms/Drupal (top 1M), or 1 in 408 for the entire internet
- W3Techs: 1 in 111 according to https://w3techs.com/technologies/details/cm-drupal
Even if we talk only about the Top 10K sites, which is where I think the "1 in 8" (12.5%) value comes from. This is no longer true, and it is now closer to "1 in 16", being 6.46% at this moment (https://trends.builtwith.com/cms/Drupal). The last time that the 12.5% value was true for the Top10K was April 2022: https://web.archive.org/web/20220417100720/https://trends.builtwith.com/...
I think it is actually time to re-activate ✨ Replace .info.yml with composer.json for extensions Postponed and deprecate the .info.yml files, so that Drupal 12 relies exclusively on the composer.json file for all info that is usually in the .info.yml.
Move the link to PHP supported versions, to make clear that we're discussing different concepts in this page (feature and security support by PHP developers vs Drupal support for that version).
Mostly because Drupal packager will introduce that automatically, and having it in two places can lead to those values becoming inconsistent with each other. Most modules I see rely on the packager to add that, and unless something breaks it shouldn't really be added to the composer.json file.
Can it be that somehow you are overwriting the user data tables? TFA stores the user secret in that table..
The change request now contains information on the different row formats and which is the default one.
I don't think that this should really need a test to check if Drupal works with other row formats. This is simply passed on to the database server when creating a table, and we would only be testing database engine functionality and not Drupal. It is the responsibility of the database engine to support the different row formats and make that transparent when that table is used.
Duplicate of 💬 Only one 'enable_page_level_ads' allowed per page Active
jcnventura → made their first commit to this issue’s fork.
Can you try the MR?
jcnventura → made their first commit to this issue’s fork.
I have serious reservations on adding support for a library that would create future maintenance problems whenever 3.x stops being compatible.
Can we maybe take a look at https://github.com/matomo-org/device-detector
Please base it on the current state of 2.x
At this moment, the main objective is to make sure that the 3.x branch works on Drupal 11. I'd love that someone that actively used this module would be interested in taking it over. One of my objectives with the 3.x branch would use the code from https://oauth2-client.thephpleague.com/ in order to have a sane codebase. Not sure if that objective is realistic, as the small add-ons for OIDC added to this module may not be possible with the pure OAuth 2.0 code that that package uses.
The CR that introduced this is https://www.drupal.org/node/3404140 →
This means that with this change, we need to remove support for Drupal 9, and have it only ^10.2 || ^11.0
Weirdly enough, when searching the core issue queue for "passkey", I got zero results. Seems like Drupal is truly stuck in the early 2000s. I've created issue ✨ Support passkey and any other login flow that is more than username+password Active to attempt to finally do something about it.
jcnventura → created an issue.
Indeed, core's user login should be a pluggable system, with core providing a simple plugin: password login. The password plug-in would take over the existing password flow (including password reset), but leave the ability to have other types of logins. The TFA login plug-in would then probably extend a good part of the password login, or clone it and modify it.
This would leave open the door to other types of plugins like SSO and passkey where the concept of password (or TFA) does not exist on the Drupal side.
Not a maintainer. Just a user of the module, and saw there was this one RTBC issue. Feel free to open again, and make it "Active" or "Needs Review", and move the version to 2.0.x-dev
The Drupal 7 version of this module is no longer supported. It's outdated because of that.
Let's keep this here, it makes no sense now to move this back to "Fixed".
Duplicate of 📌 Automated Drupal 11 compatibility fixes for openid_connect Needs work
It is clear that the maintainers no longer use Drupal 7. I'd recommend - if you want the 15 or so RTBC issues to be merged - to apply to be a co-maintainer for the Drupal 7 branch.. Although, 2 months from now, I suspect that Drupal.org will mark all Drupal 7 projects as unsupported, and probably start dismantling the D7 infrastructure.
jcnventura → made their first commit to this issue’s fork.
Can you provide more information? The logo.png file is 512x512px and 3.5Kb in size, exactly as you request.
Is there anything in your request that is not met by our existing logo.png file?
Please don't comment on a closed issue. Especially since 🐛 Menu link translations can not be cteated Active exists and may fix this issue.
Marking this as a duplicate of 📌 Public followup for SA-CONTRIB-2024-043 Postponed
2.0 alpha is somewhat affected by some known security issues. And the fix for the most recent one hasn't been merged in yet, so that is another issue still pending.
So, camslice and avo webworks, do you still believe that this still needs a review? What I read from your non-change of status is that you tested, and think it works, but you consider that you didn't test it completely.
Please refer to https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → for what the status field in an issue means.
If the compatibility problem is arising here, there is not much that can be done. This login controller is not used for the normal functioning of the module, but as a lock to prevent use of the user.login.http REST API. This because we haven't found a way to request TFA over the REST API, so we gave up and simply locked that path completely, so as to not open a security hole by allowing TFA to be bypassed completely on that method. If you want to skip this horrible, horrible kludge simply use branch 2.x of the module, in which this was never added (and is of course totally open to TFA bypass...).
If you want a site with this TFA module that is not trivially bypassed via REST API, it simply can't be decoupled. Maybe other TFA modules have better solutions to this, and we would LOVE to have help in moving the 2.x branch to an event-based solution. That solution would not rely on overwriting the login form (and controller in this case), but would actually hook into every request and insert a few additional steps on some cases (login via /user/login form, login via REST API, login via smoke signals and password reset), to request the TFA token from the user in those cases.
At one point, that interface did allow FALSE to be returned, and that created some issues, as there is really no circumstance where that should happen on the call to decrypt(). It either decrypts and the decrypted string is returned or it doesn't decrypt, and an exception should be raised. It was found that sometimes developers wanted a "soft" exception and returned FALSE, and other times threw the exception.
However, calling decrypt("") is really an edge case here, as it might actually be that the correct answer to that is to return back the empty string. That was what I suggested in #4.
Ideally, the tfa module that @alexharries referred to in the issue summary should be modified to somehow handle the exception or simply prevent calling the function with an empty string, which I believe is done (see https://git.drupalcode.org/project/tfa/-/blame/8.x-1.x/src/Plugin/TfaVal...).
Adding the Real AES logo issue as related, as I think that these 2 logos should be related.
We need to be realistic about the fact that 90% users of the encrypt module (24183 as per https://www.drupal.org/project/usage/encrypt → ) use the encryption provided by the real_aes module (21953 as per https://www.drupal.org/project/usage/real_aes → ).
As such, these two projects should use a common design for their logo. I'd urge whoever is creating the logo here to take that into account, and think about other encrypt plugins should adapt the logo.
This honestly should be a derivative of whatever the Encrypt module uses, as Real AES is just an encrypt plugin. Let's postpone this until 📌 Project Browser: Create a logo for Encrypt Needs work is solved.
What exactly did you type in the Redirect Login configuration? It seems we need to validate that it is an internal path....
The changes are welcome, but need further work before being merged:
- Declaring a module to be compatible with
=> 10.1means that the next version of the module will be compatible with Drupal 12, 13, 14, 15, etc.. There needs to be an upper bound, which is why most maintainers use^10.1 || ^11 - As per the comment, it seems that
getPrefixInfoneeds to be removed, since #2223073: Calling DatabaseSchema::getPrefixInfo() on a non-default connection returns the wrong database - write tests → has now been fixed in core
Taking into account the fact that rector didn't find anything else to be fixed (https://dev.acquia.com/drupal11/deprecation_status/projects/encrypt) and that this only changes tests, and they are now passing, I think this can be merged.
Marking RTBC even if I was the last one committing, since my change was really minor.
Test modules should not have a core_version_requirement as long as they are in package: Testing. See
https://www.drupal.org/node/3070687#test →
Also, if possible change the default branch in Gitlab to 5.0.x, please.
jcnventura → created an issue.
Before you tag the next version you should also address and fix the following:
@joseph.olstad, I think for CDNJS you will have to get support from the Bootstrap team to publish under their organization. Considering that HeroDevs has somehow gotten some support to be their Bootstrap 3 long-term support partner, you might need to somehow have that partnership cancelled. See https://github.com/cdnjs/cdnjs/discussions/14191
As to jsDeliver, you can try the following: https://medium.com/javarevisited/how-to-host-your-repository-js-css-on-o... (not sure if it is still applicable).
I don't think this will ever be possible, to be honest. The version of this theme for Bootstrap 5 is basically the Boostrap Barrio theme. I think the entire premise of how the themes are built is simply different. But it would be good to see some at least some documentation on how to approach this problem.
Making clear what this is all about, now that the theme already supports Drupal 11 in the 5.0.x branch
I fully support the upgrade to use bootstrap 5. There is a reason why Bootstrap 3 is unsupported for more than 5 years and I'm not sure if @joseph.olstad is fully aware of the security implications of his attempt to create Bootstrap 3.4.2. HeroDevs has a Bootstrap NES version where they fix the security problems in older versions of Bootstrap and they are multiple devs being paid by a subscription model to keep that up (for a very small list of CVEs fixed in Bootstrap 3 in July 2024, see https://www.herodevs.com/support/nes-bootstrap?utm_source=Bootstrap_site...), and yes they also have a jQuery NES. I honestly believe that there should never be a supported version of the 3.x branch of this theme that is not an alpha (i.e. 8.x-3.33-alpha1, etc.), to make clear to everyone using that effort that they really have no security coverage and should think about migrating away from Bootstrap 3.
The question however, is how is the upgrade process between version 8.x-3.32 and 5.0.0-beta1? I'm pretty sure it is not a simple step of changing the version number in composer.json...
At this moment, and since this is only changing the tests directory, removing support for D8 and D9 would not be entirely justified.
There's still about 170K sites out there running D8 and D9, and maybe they can't upgrade to the next major for some reason, but could keep up with this module.
@joseph.olstad Think about never actually doing any release other than alpha1, alpha2, etc. There are known security vulnerabilities that have gone unpatched for the last 5 years.. If you're going to make new versions, you need to decide if you take the responsibility to fix those vulnerabilities or if you simply ignore them. Marking them as alpha clearly signals those releases as unsupported.
@joseph.olstad in #23 (which is weirdly unpublished), you suggested to fork twbs/bootstrap and have the build method be a Drupal method. You should be fine with just adding GitHub Actions to the fork you just did in GitHub to generate the minified Javascript libraries. Then, the only change would be to the libraries.yml of this theme to fetch the libraries from GitHub.
There's zero need to create a Drupal module when the code you forked does not use any part of Drupal.
The jQuery 4 compatible version of Bootstrap should not be a Drupal extension at all. It should probably be in GitHub and a simply be a fork of the existing sources.
@joseph.olstad in #10 was the point where I got this theme to work in Drupal 11. The upstream bootstrap code has a test early on that checks for the version of jQuery being used. If it detects jQuery 4, it throws an error and refuses to run.
My efforts stopped at that point, as asking a Drupal theme maintainer to co-maintain Bootstrap 3 is simply too much to ask (even if it is maintaining a patch and not the full library, I think it will eventually become the full library).
We have 2 years to migrate away from Bootstrap 3. I think the best solution here would be a tutorial on the best way to migrate from this theme to another of the Bootstrap 5-based themes.
Yes, we don't use those for anything else, and I assume the free tier will be enough...
Once you accept the invite, I guess you'll have the access for that :)
Also, interesting bit, @joseph.olstad is one of the co-maintainers..
This needs a lot of work, and not simply adding "^10 || ^11".
First of all, it seems that it doesn't work in PHP 8. That basically means that it doesn't even run in Drupal 10. We need 🐛 dmu_upgrade not compatible with PHP 8.0+ Needs review to be RTBC and merged. We also need 📌 Make the code generated Drupal 10 compatible Active to be fixed.
I don't use this module anymore, as I don't have any D7 custom code. @gábor hojtsy was very interested in this at one time. Not sure if Acquia uses anything from this module in https://www.drupal.org/project/acquia_migrate → . If any of you is interested in maintaining this, please do fix the above issues, and I'll grant you co-admin rights to tag new releases.
Please do a pull request on upstream pharborist.
Indeed. Looks perfect. Thanks for your help @Sourojeet
Please don't use the patch. MR113 is the current state of the work on this. And it is still under "Needs review" status.
jcnventura → made their first commit to this issue’s fork.
First, to clarify that the original maintainer is @yannickoo.
Second, this change has security implications that may not be transparent to site administrators. With this, the person responsible for managing the menu can now define both where an upload is added and the file type. I don't think it would be too hard to use this to now inject a malicious JS or PHP file in the system somewhere where it is an XSS or code injection problem.
I know it is a pain to maintain, but this should really use a hard-coded allowlist of possible extensions (jpg, jpeg, gif, png, svg, etc.) and only apply those extensions that are in the whitelist.
Also note that this may not be compatible with Drupal 11: https://www.drupal.org/node/3363700 →
This coupled to the fact that the issue summary doesn't even say why this would be a nice feature to have leave me with a lot of reservations on whether this is RTBC. I'm setting this to needs work, so that we are sure to have a system that works with the new file.validator service, and also to somehow block the security issue that I believe this is introducing.
Thanks for this. Even if it made sense 2.5 years ago, and not so much anymore, as PHP 7 is no longer supported by anyone (neither Drupal nor PHP itself).
jcnventura → made their first commit to this issue’s fork.
There needs to be a better way to pass that test than to force us to now track webform versions in the composer.json.
Worst case, disabling phpstan, but I think the best would be to add a phpstan.neon file to ignore this one.
At first glance, no but I should check if any of these allow the creation of the feed node:
- Entity Reference reference access (ReferenceAccess)
- Entity Reference valid reference (ValidReference)
RTBC as per #9
@japerry, please contact also @adriancid who is the one that has created all the releases in 2023.
There seems to be a very minor phpcs bug in the CI. Maybe we can fix that before tagging it
Should be good now. No one raised any bug and it's been a month since alpha3
All these deprecations really make it seem like the version for Drupal 11 should have a new major.
The Drupal 11 MR is now broken, and I don't think it makes sense to fix that before this is committed, so RTBC++.
And please close MR 171 in this issue, so that it doesn't pollute the MR list.
valthebald → credited jcnventura → .
PHP uses short-circuit operators in && operators. The only way to get to the second operator is if the first one is evaluated to true. In this case, since the error is complaining about it being a boolean, it means that $userinfo is TRUE.
It is hard to understand what the origin of the problem is here, since the error is in line 273 of OpenIDConnectSettingsForm, specifically in the buildForm() method, but that method is defined in lines 123 to 272 (see https://git.drupalcode.org/project/openid_connect/-/blob/8.x-1.4/src/For...)
Can you please confirm the version of the module that you're running, and what patches you're applying to it?
They were declared before in bootstrap.libraries.yml and were removed when the MR was merged. They should be able to be trivially added back, as they are now all provided by the (non-core) jquery_ui module.
This should be merged ASAP, since Drupal CI will be discontinued in 4 days.
Weird, the post-commit build failed with the following error:
"ERROR: Uploading artifacts as "archive" to coordinator... 413 Request Entity Too Large id=1953767 responseStatus=413 Request Entity Too Large status=413 token=glcbt-64
FATAL: too large"
But only the next minor.. Is there a way to request a retest?
The good thing is that this feature seems to be used in 94% of the current browser market share: https://caniuse.com/?search=one-time-code
I'd say that is enough to consider this ready for being used.
@finnsky, I agree with you but Bootstrap 3 has been EoL already for 5 years. The real news for those wanting to upgrade to Drupal 11 when it is released in about 1 month, is that this theme is simply not compatible with that version of core.
The theme needs to clearly document that, and say that anyone still using this theme (which still works) will now be locked into Drupal 10 until they can migrate away to another theme. The fact that Bootstrap 3 has been EoL is not in the same level of importance.
Because this theme requires Bootstrap 3, which is no longer supported, and in turn Bootstrap 3 requires a version of jQuery less than jQuery 4 which is shipped with Drupal 11, this theme will become end-of-life when Drupal 10 itself reaches end-of-life (currently scheduled for mid-2026).
This should be clearly documented in the module homepage, moving the recommendation of Bootstrap 5-based themes from the bottom of the homepage to the top, and acknowledging both the fact that Bootstrap 3 is no longer supported, and that the theme will never support any version of Drupal greater or equal to 11.