Cleaning up tags.
I also feel "needs framework manager review" can go since @smustgrave and @catch have checked this.
Made a change record:
https://www.drupal.org/node/3567198 →
Added "Release notes snippet".
Full transparency: as English is not my first language I did use an LLM to improve my writing to something more formal.
Moving this back to RTBC, I hope I this is the right place.
I made an MR against 11.x
It's nothing new, it's the same patch from 4 years ago, superb work from @gungwang!
@cilefen you added no comments as to why this needs work, so I'm bumping it to needs review.
The patch works and does solve the issue. In our case it was also a migration that created revisions. Languages are enabled but the problem exists even without translations in place.
I wouldn't mind maintaining contribute if it's going to be part of Drupal CMS, opting for Security Support, slimming it down to what is absolutely needed and getting feedback from the community.
I've just yesterday got news
It is 8 years since I made this post, but I still back the idea of it being part of Drupal.
So, whether in Contribute or "Drupal Association Extras" or elsewhere I could devote 6-10h a month to this, even a bit more upfront to get things stabilized.
This page is about miniOrange services, not about generic 2FA. Update title to reflect that.
Update title to reflect content and avoid people landing here when they don't need/use miniorange.
I'm failing to fix this properly, mainly because many tests need to be updated for the new permissions, but that generates a very important question:
Do we update the tests to assume the new permissions are in place or do we update the tests to assume they don't have access to usernames?
I'm gonna stop here and wait for some feedback.
I made an attempt and fixed the failing PHPUnit test. There are 2 more now but they seem related to gitlab artifacts and not to the changes in code.
I'm trying to understand what this is about.
Getting the cancelation button to work with HTMX is quite hard.
I need to either remove or end with a hybrid option that is not entirely HTMX.
I am not sure which direction I'll pick. Feedback is most welcome, especially from people who use the cancel option.
Improved logic and order of checks, for better code readability. Also fixed CS issue added previously and adapted previous changes for new user registrations.
@vensires the idea behing AND is to do things the same as with emails:
- right now you are not allowed to view the email unless you have BOTH `view user email` AND `access user profiles`
I do not object to the idea of an update hook to give `view user names` to all people, however since this is actually an open security-related issue where usernames are exposed in JSON:API I would prefer not to do so, not yet. Many of the sites that have JSON:API enabled might not know that usernames are exposed. So it would be better for us to apply the fix, rather than apply the fix and at same time undo it by giving anonymous users the permission to see usernames.
Open to discussion of course, as always.
I'll bump this to needs review, mostly because I'd love a fresh set of eyes to check it before I continue working on it again.
Thanks
Thanks for the feedback @vensires! Much appreciated.
I'm assigning the issue to me for now, but it might be a couple of days before I can jump to it again.
If anyone is willing to get this further by all means, be my guest.
@vensires from a quick overview I believe you got valid points, let me check them in detail and I'll get back to you with improvements.
Pushed an extra fix towards the direction @arunsahijpal suggested.
I'm taking a shot at this. I hid the old MR because it was 4 years old and I didn't feel like updating it.
Made a new MR (https://git.drupalcode.org/project/drupal/-/merge_requests/13617):
adds a new permission: View usernames
copies logic from emails [#3070293]
adds tests (had help from Claude for the tests)
In my manual tests this works nicely for JSON:API, hiding usernames unless the new permission is granted.
Note:
Permission to View user information to anons allows them access to example.site/user/2 which could easily expose the username in the theme. This is not addressed in the MR and is not necessarily a core security issue.
Not much I can do about it, themers must check for permissions before rendering the usernames in that case. The work in the MR does help in that direction but doesn't solve that case, it can't.
Please have a look
bserem → changed the visibility of the branch 3240913-information-stored-in to hidden.
I've only now realized that comments submitted in gitlab might not be visible here according to user settings. Work has been done in https://git.drupalcode.org/project/drupal/-/merge_requests/13399#note_60... and screenshots have been added in that comment to visualize the changes.
Quoting gitlab comment here, without screenshots, for clarity:
Thanks @poker10 for the review. Your concern is valid, so I updated the robots.txt file to disallow /search/ (as it was) and /search? (new entry).
Updated some texts in MR#52 and added an update hook to make the whole process easier for site builders.
Tried https://git.drupalcode.org/project/image_widget_crop/-/merge_requests/52... against 3.0.x and it works.
I would advise we make an update_hook to set the library settings (from the modules configuration) to:
https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.6.2/cropper.min.js
https://cdnjs.cloudflare.com/ajax/libs/cropperjs/1.6.2/cropper.min.css
otherwise people will land with a non-working cropper after the update.
This issue was a worth opponent. In my case it was caused by the "rabbit_hole" form displayed on the sidebar of node add/edit pages.
This commit https://git.drupalcode.org/project/rabbit_hole/-/commit/18b07d6c45f0bcc7... (committed in a dev version) was all it took to fix things.
I'm pasting it here so that other people get to a fix faster than me.
The route definition at /admin/content/feedback/{id}/delete requires the id parameter (with regex validation ^\d+$), the parameter
will always be provided, so I removed the default NULL value entirely. The parameter is now simply string $id.
New -dev version released, please report any findings.
Thanks
I am not sure whether making ID nullable is the way to go here.
This comes from https://git.drupalcode.org/project/admin_feedback/-/commit/8cb1c86229df8...
and
https://www.drupal.org/project/admin_feedback/issues/3070217 →
It is possible that max-ar is right and we don't need the id in the first place.
Anyone willing to work on this?
Merged, it's now on dev.
Will issue a new release before the end of the month.
Thanks all for your help on this one. Much appreciated!
Recreated the branch and the MR to not include any 11.2.x stuff as advised in #7 above.
The diff is now clean. Marking as needs review again.
Downgrading psr/http-message (2.0 => 1.1) solves the issue and it is highly probable you don't really need the new updated version.
Use composer why psr/http-message to figure out which module requires what and if you are blocked from using the older one for any reason.
Thanks for the pointers @smustgrave
- Updated the target branch
- Hid the MR towards Drupal 10
- I am not allowed to delete my branches from gitlab
Having an extra entry is also an option, I opted against it for less repetition, as the current change achieves the same results.
Putting back to needs review.
bserem → changed the visibility of the branch 3550083-robotstxt-search-d10 to hidden.
I've made the batch size configurable.
Anyone care to review before we merge and publish a new release?
Thanks all
@ressa I'd be honored to help here and I'm honored by your suggestion. Right now I've applied for a different position inside drupal.org which would demand a lot of my time if it becomes a reality.
As I hate to start things and not finish them I'll wait for the other process to come to a conclusion before I apply here. I should know in the next couple of weeks.
Meanwhile, lets get @layalk the tools she needs to get that translation going.
First of all let me clarify that I'm not a member of the Arabic translation group, but I've been around translations and I'm the manager of the Greek group for too many years (way too many) so I have very good idea about how things work with translation teams. You need someone dedicated to handle things.
Knowing @layalk I can vouch for her reviving the Arabic group. Translations are always hard and people often lose interest after their project is done, especially in languages where Drupal is still trying to get momentum. On the other hand, Drupal is losing ground especially because the translations are not readily available. It's a vicious cycle.
Layal has been in the Drupal community for years and is as committed as ever. If anyone can revive that group and move things forward it's her.
I fully suggest we proceed with the management change.
@vensires have a look at https://git.drupalcode.org/project/drupal/-/commit/6810f24aef13523c17d35...
@alexpott @heddn the committed change only alters the error message, sadly it doesn't solve the error message.
PS: it looks like you didn't like me highlighting the issue in the first place. I can't explain why I'm omitted from the credits.
@greg FYI the patch from gitlab applies nicely to 10.4.8
Thanks for the clarifications all
@vvuksan-fastly please forgive me for re-opening this, but I understand closed issues won't get much attention.
Fastly is definitely a niche module and doesn't apply to all projects, but I do agree with all of the above people. Maintaining a split has always proven to be cumbersome and it is something we prefer to avoid for simple things.
AdvAgg, another performance related module supports disabling the module on settings.php level (source: https://git.drupalcode.org/project/advagg/-/blob/5.0.x/src/Form/Settings....
In reality, Fastly is something you will only need on a live environment. It would be great to be able to do just that on module level, and not require an overly complicated extra module.
Again, forgive me for opening this again, feel free to close it again and I believe we'll all understand and stop requesting this. I certainly hope you reconsider though.
Thanks
New MR against 2.2.x, new patch attached for use with composer, same state as MR (from https://git.drupalcode.org/project/honeypot/-/merge_requests/60.patch)
Attaching new patch.
Notes:
- I would love to bump hook_update_N to 8201 (from 8105) to match with version 2.2.x
- Can't update the MR as the forks don't have 2.2.x and won't sync themselves
- Please add https://www.drupal.org/u/jaims-dev → to credits, as he helped with the new patch
@nicxvan added some feedback as a gitlab suggestion
Much appreciated mate!
Have a nice day
It's good to know it wasn't intentional, that means a lot :)
As a maintainer you can see at the bottom of the drupal issue (above the green action buttons) the "Credit & Committing" section.
In there you should have an option to check/uncheck who participated.
I am adding a screenshot of this area from a different module, just to highlight how it looks like.
You can chose whoever you want. It is customary to pick people who contributed with a patch and/or testing notes (even if not providing code patches).
Once you fix the issue, and after 14days pass when it gets closed, people will get credit.
It also gives you the commit message if needed, but I believe it is enough to just do the checkboxes.
@lobsterr it would be wise to grant credit to the people who have worked on this over the last 7 years and provided fixes to the issue by contributing patches until one of the maintainers sat down to deal with the issue.
Crediting yourself alone is not the way to encourage people to work on open source projects.
Running the latest dev version (almost equal to 4.3.7) I am getting these reported too but properly categorized under ignore in both the UI and drush.
$ drush us-a weather
[notice] Processing /var/www/html/web/themes/custom/weather.
================================================================================
Weather, --
Scanned on Mon, 26/05/2025 - 08:50
FILE: web/themes/custom/weather/templates/form/select.html.twig
STATUS LINE MESSAGE
--------------------------------------------------------------------------------
Ignore 13 Since twig/twig 3.12: Twig Filter "spaceless" is deprecated.
See https://drupal.org/node/3071078.
--------------------------------------------------------------------------------
Closing this
Thanks @ki
Let's hope this helps people out there even more.
I can verify that it works as expected with the latest dev version!
@ki I'm glad you like the initiative! It means a lot.
Regarding the new colors committed it looks like the environment are indistinguishable for people with Protanopia and/or Deuteranopia (and Tritanopia for that fact!).
This is how things look with the current dev version:
You can see that:
Protanopia and Deuteranopia affected people can't tell the difference between Local+Dev and Stage+Live.
Tritanopia people can't tell the difference between Local+Stage.
#AD1818 suggested by @gkffzs also has the same issue.
I am testing with https://davidmathlogic.com/colorblind/#%237B1DD3-%230057AD-%23486119-%23...
The colors are originally suggested are just a bit better on this aspect:
Here is the link to the comparison tool: https://davidmathlogic.com/colorblind/#%234A0080-%23005B94-%2359590D-%23...
The colors that were used up to now had no such problem:
but they did suffer from low contrast scores against white... (comparison: https://davidmathlogic.com/colorblind/#%234A0080-%231E90FF-%23DAA520-%23...)
So, the latest commit solves the contrast issue, but adds other issues.
I'd suggest we change them, either in this or make a new one and address color-blindness there.
Closing this as it for Drupal 7.
@gkffzs adding another environment type ('lo', 'dd') should be a different task and not part of this one.
Here I am only trying to make sure that the current codebase is a11y-compliant
MR !6 doesn't apply properly (patching via composer). MR !4 does
vensires → credited bserem → .
Uploading new patch for current version of JS file for those who need it.
We need to think if this has any security implications. If a user with permissions to edit the message adds malicious scripts?
There need to be some checks if it is not just text anymore.
Logo added
New functionality added, some areas also got improved down the line.
A new release will happen soon
This is now merged, please use dev version of the module until a new release is made
Thanks a lot for this, and for Drupal.theme()
bserem → changed the visibility of the branch 3522968-add-option-to to hidden.
bserem → changed the visibility of the branch delete_all_node_feedback to active.
bserem → changed the visibility of the branch delete_all_node_feedback to hidden.
bserem → created an issue.