Problem/Motivation

Following the recent removal of the Toolbar module from the main core development branch (D12), with change records:
In D12: https://www.drupal.org/node/3623310
and deprecated in 11.5.x
https://www.drupal.org/node/3616232

We should now be able to completely move over the core Toolbar module's code base to this contrib module.

Additionally, based on the answers at #3484850-58: [meta] Tasks to deprecate Toolbar module, the minimum requirement to support would be 11.5, but ideally, we would like to potentially support 11.4 as well, so users could have a bit of a wider window to upgrade.

Steps to reproduce

Install the module on D11.4.
The module should work fine, as it currently does with 11.x-dev (11.5), prior to the removal of the module.

Proposed resolution

Expected work : Mostly :

  1. Merge over all the revisions after May 15 2026 (last synchronized revision) from the core Toolbar module Git tree, into module repository's 1.x branch. If possible, try keeping as much of the original history tree as possible with the authors, dates, etc.. with the commits from the Drupal core repository.
  2. Update the core version requirements to : ^11.4 || ^12

    3. .

Once these changes have been reviewed, tested and approved, merge them in the 1.x branch and create an initial 1.0.0 stable release.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork toolbar-3624202

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dydave created an issue. See original summary.

dydave’s picture

dydave commented
Issue summary: View changes

dydave opened merge request !2

ressa’s picture

ressa
he/him
commented
Status: Active » Needs work

Thanks for fast action @dydave! I tested the MR and it works great.

Drupal core Toolbar already installed = a replacement situation

  1. Installed a fresh Drupal 11.4.7 instance.
  2. Installed Admin Toolbar and Drupal core Toolbar.
  3. Git cloned Toolbar from https://www.drupal.org/project/toolbar/git-instructions into web/modules/contrib, and checked out the dev-branch.
  4. Flushed all caches.
  5. Refreshed the modules listed on the Extends page, and Toolbar disappeared from under "Core" and was shown under "User interface" instead, so it appeared that the contrib version had now taken over.
  6. Clicked around in the interface with Admin Toolbar, flushed caches, etc. and everything worked well.

I did encounter a problem ...

"Fresh" installation of Admin Toolbar with contrib Toolbar

If I install the Admin Toolbar module with contrib Toolbar, I get this error:

The website encountered an unexpected error. Try again later.

InvalidArgumentException: The callable definition provided was invalid. Either class "Drupal\toolbar\Hook\ToolbarHooks" does not have a method "toolbarAlter", or it is not callable. in Drupal\Core\Utility\CallableResolver->getCallableFromDefinition() (line 102 of core/lib/Drupal/Core/Utility/CallableResolver.php).

I also tried deleting the module from Drupal core (the web/core/modules toolbar folder) and got a similar error. Though I am not sure if that is taking the testing too far? :)

Should I also try with Drupal 12 Alpha?

dydave’s picture

dydave commented

Well received @ressa!!
Thanks a LOT!!

Let me look at this issue ASAP.

Should I also try with Drupal 12 Alpha?

YES please!! 😊
In the meantime, could you please help me with the testing on D12?

I'll get back to you on the AT issue ASAP.

Thanks again @ressa!!

smustgrave’s picture

smustgrave commented

This is a massive MR did you do the subtree split from? https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... typically for these initial stable release it's just about adding composer.json, gitlab file, and removing the deprecation link.

dydave’s picture

dydave commented

Thanks @smustgrave!

This is a massive MR did you do the subtree split from?

YES, indeed, I'm quite happy I was able to bring over the code base with its full history, with the command:
git filter-repo --subdirectory-filter core/modules/toolbar/

typically for these initial stable release it's just about adding composer.json, gitlab file, and removing the deprecation link.

I did that a while back already in July, see the current 1.x commits:
https://git.drupalcode.org/project/toolbar/-/commits/1.x?ref_type=HEADS
with the first commit listed.

Same thing here: I brought over the full git tree from the core toolbar module.

This merge request:

  • Updates the 1.x branch with the latest commits from core.
  • Updates the core version requirements to support 11.4 instead of 11.5, therefore:
    The code of the module from 11.x is not completely aligned with the rest of core's code base on 11.4.x: For example, all the dependencies to the toolbar module were removed from other core modules in the 11.x branch, which is not the case in 11.4.x.
  • Bonus (could be removed): Enabled services autowiring.

Thanks in advance for your reviews and comments.

dydave’s picture

dydave commented

@ressa: I'm not reproducing locally 😖

I'm afraid, I'm going to need more info...

What version of AT did you use?
Did you only enable AT or any other modules, such as AT_tools and/or AT_search?

Could you try reproducing the error and check in the recent log messages (dblog) if you can find the full message, if possible with a more complete stack trace?

I'll trying trying different things to see if I can reproduce the error you encountered.
Thanks again!

ressa’s picture

ressa
he/him
commented
Status: Needs work » Needs review

Hi @dydave, good news! I did some more experimentation, and it looks like it is because some internal paths are kept in the server memory, and not even removed when you flush the caches, except if you add some "extra" steps, like described in How to fix "The following module is missing from the file system..." warning messages > You moved the module inside your Drupal installation adding $settings['class_loader_auto_detect'] = FALSE; in settings.php. Then, if you run drush cache:rebuild, all is well.

Now, with that added you can add or remove the contrib Toolbar module from web/modules/contrib, flush caches, and all is well. Another option is to restart DDEV. For more, see #3540339: Rebuild all caches, including APCu, when clicking "Clear all caches".

Since it seems to happen mostly if you transition from contrib to core module (not a likely scenario) I don't think contrib Toolbar needs to worry about this, right? It could get mentioned in the release note, something like this? I

If you encounter errors like these, rebuilding caches thoroughly may be needed:

The website encountered an unexpected error. Try again later.
Error: Class "Drupal\toolbar\PageCache\AllowToolbarPath" not found [...]
InvalidArgumentException: The callable definition provided was invalid. Either class "Drupal\toolbar\Hook\ToolbarHooks" [...]

Anyway, the code seems to work well in Drupal 11, so setting Status back to review of MR for Drupal 12 Alpha :)

PS. I forgot to include in my comment #4 that I install all three AT modules: drush in admin_toolbar admin_toolbar_tools admin_toolbar_search toolbar && drush un navigation

smustgrave’s picture

smustgrave commented

If you’ve done the subtree split in July a lot of additional commits have happened since then. I’d highly recommend doing it again.

Still don’t know why the MR is showing so much? Was there a branch here before? If so a new dev branch should be created.

Example of claro stable release notice made a 3.0x with the sub split and this MR is much smaller for a stable contrib release https://git.drupalcode.org/project/claro/-/merge_requests/5/diffs#b49f34...

Hope that helps

smustgrave’s picture

smustgrave commented

If you’re on slack happy to carve some time to help next week

ressa’s picture

ressa
he/him
commented

Thanks for the feedback @smustgrave, and offer of assistance.

Since my last comment, I tested the MR and here are my observations, in case you two decide to not change the code base, and use the current MR:

I tried contrib Toolbar in Drupal 12 Alpha (without Admin Toolbar, since it's not D12-ready yet) and I couldn't find any problems, everything looks fine :) I compared it side-by-side with a Drupal 11 installation using core Toolbar, and didn't see any difference, apart from the new "Welcome" menu item, and compared to the original:

  • Visually, they look the same
  • Mobile view looks good and works well
  • Switching between Horizontal and Vertical mode looks fine and works well
  • Breakpoints behave identically

So it looks and behaves identical to D11 core Toolbar, and seems Drupal 12-ready.

smustgrave’s picture

smustgrave commented

Didn’t test functionality my concern is what the MR is showing. All the other deprecated modules and themes to my knowledge when doing the setup stable release tickets didn’t show all the changes like this. So it may work but just concerned something got borked

ressa’s picture

ressa
he/him
commented

Yes, and thank for voicing your concern, I do understand why you're puzzled by the many changes, since usually there are less ... Reading my comment again, it wasn't totally clear, but let me emphasise that I very much appreciate you raising and sharing your concern, it shows that you care.

It just happened, that while I did the Drupal 12-test, you posted your comment, and I decided to share my observations anyway -- where I could have just posted, "Thanks, looking forward to what you two decide." :)

smustgrave’s picture

smustgrave commented

@ressa you're good and I'm always happy to help :)

This would be my suggestion.

1. I think you almost have to do the subtree split again. If you did it in July then you're definitely missing a number of commits. Not sure if that's what the bulk of those changes are bringing over but seems like it would be muddying the commit history. A fresh split should fix that.
2. I checked there wasn't a branch before so once you do the subtree split you should push that branch up to 1.x fully, 0 changes.
3. In this ticket do the bare minimum to get a stable release (composer.json file, gitlab, small changes to .info, and drop the deprecation attributes from tests). If you add more then again the commit history gets muddy. So again recommendation but example don't do OOP hooks in this ticket if it didn't happen in core.
* What I've done for all the modules/themes I've taken from core I do a branch that's a carbon copy that can be dropped in place. Then I start a new branch for dev work going forward. This way I don't break anything but fixes can land in a new branch and then sites will have to make the choice to update (this case 2.x). But that's fully up to you just how I do it.

@ressa already confirmed nothing broke but concern is if you start making changes immediately from core and something breaks in contrib that wasn't in core, users are now blocked from updating core.

Think if you do those steps then this MR will be minimal and easy to review and covers the requirement to get a stable release out. Additional work can happen separately (same branch or new one fully what you decide)

Decision is up to you not trying to tell you what you must do so if I come off like that I apologize but current MR I don't think is a clean git history from core.

dydave’s picture

dydave commented

Super nice @smustgrave and @ressa!
Thanks a lot!

Yeah ... I was wondering whether it would be better to start over and do a clean split....

No problem, I'll do that and make all the changes or steps you suggested.

I'll follow all your advice carefully and test again with the steps suggested by @ressa.

I'll keep you posted in this issue and on Slack as well.

Thanks again very much!

dydave’s picture

dydave commented

I'll be working on this today, most likely this evening.

Sorry for the delay... I've seen the updates from @drumm in the composer namespace ticket: waiting on a stable release before being able to make the change....

This is on the top of my list.

Thanks again for all your help! 🙏

dydave’s picture

dydave commented
Status: Needs review » Needs work

@smustgrave: Really sorry ... I'm a bit stuck on this step:

2. I checked there wasn't a branch before so once you do the subtree split you should push that branch up to 1.x fully, 0 changes.

I can't seem to be able to rewrite the 1.x commits history or delete the 1.x branch... Since there is already a 1.x-dev release 😖
https://git.drupalcode.org/project/toolbar/-/commits/1.x/?ref_type=heads

Additionally I already merged some changes previously, see:
https://git.drupalcode.org/project/toolbar/-/merge_requests/1
Which appears as the first commit in the 1.x list.... so if I merged changes on top of the existing ones, we would still not have a "clean" continuous history from core.

I cloned the 11.x toolbar module as you recommended in:
https://git.drupalcode.org/project/toolbar/-/tree/clone-11.x?ref_type=heads

I'm not sure exactly how to proceed from here: Could there be any way to do anything with the 1.x branch as it currently is?
The simplest option would be to delete and start over, but I'm not sure I have the permissions to do that... Can I delete the 1.x-dev release ?
https://www.drupal.org/project/toolbar/releases/1.x-dev
I keep getting my commits rejected by hooks:

remote: Resolving deltas: 100% (1812/1812), done.
remote: The branch 1.x is tied to a development release on Drupal.org, and force pushes are not allowed.
remote: error: hook declined to update refs/heads/1.x
To git.drupal.org:project/toolbar.git
 ! [remote rejected] 11.x -> 1.x (hook declined)

Otherwise, I could also roll back on my last commit in the MR, just to keep the ones from core and still merge that on top of what we currently have ...

I think creating the dev release was a mistake... I should have kept it as a development branch untied to a tarball release 😖
Is there any way I could ask instructure admins to delete the branch and release for me?

Once again, any suggestions would be greatly appreciated!
Thanks in advance!

ressa’s picture

ressa
he/him
commented

I actually thought about this yesterday @dydave, and feared that something like this could happen ...

I came to the conclusion that simply releasing the first stable release of Toolbar contrib as version 2 was the most pragmatic solution. It would of course be nice if it was 1.x, but if it's too much hassle and you can't delete 1.x for a fresh start, it wouldn't make any difference to the end users, in my opinion.

dydave’s picture

dydave commented

That's another good idea @ressa!

Otherwise, maybe we could ask in the Moderators queue?
I found this yesterday: https://www.drupal.org/node/3482472

Maybe Alberto (@avpaderno) could help us on this one?

I'm asking a bit on slack ... Let's see if we can fin a solution otherwise ... we'll try to find another solution. 😅

ressa’s picture

ressa
he/him
commented
Related issues: +#3618445: Delete old branch audit 5.x-1.x-dev

That sounds great! Though I since remembered another issue about the same thing, so I think a fresh beginning in version 2 is probably necessary.

smustgrave’s picture

smustgrave commented

Could try force pushing? But that could get messy.

Agree with just doing 2.x of a new tree split.

dydave’s picture

dydave commented

OK, this is what Fran from #infrastructure told me in Slack:

I’m afraid it’s not possible to delete releases once they have been created.

why not creating a 2.x from scratch and mark 1.x as unsupported? that’d be the easiest.

Two solution at this point:
1 - We stick with the current MR since it's working, etc... But might be a bit "muddied" as @smustgrave said ... I could roll-back on some of the changes, for example the Autowiring ==> No problem.
So we could leave really the bare minimum.
2 - Start over the whole process with the 2.x branch and mark 1.x as unsupported.
Very simple, I could just clone the branch:
https://git.drupalcode.org/project/toolbar/-/tree/clone-11.x?ref_type=heads
which contains the vanilla Toolbar from core.
Then add the other files and changes from the other branches into a clean MR.

Which option do you think we should follow?

Thanks again for all your help! 🙏

dydave’s picture

dydave commented

OK, looks like you answered while I was talking to Fran in Slack.

So we're OK for option 2 ==> Go with 2.x ... sounds good!!

I'll get the changes prepared as soon as possible later today.

Thanks again very much for your help!

dydave’s picture

dydave commented

@ressa, @smustgrave :

Just thought about this :

Could we use 1.0.x ?
To release 1.0.0 ?

Or should we stop thinking and just go for 2.x?

Thanks in advance!

smustgrave’s picture

smustgrave commented

Think it’s up to you. There’s no issue going straight to 2.0x

ressa’s picture

ressa
he/him
commented

@dydave I think going straight for version 2 makes most sense, for a clean slate with no cruft.

dydave’s picture

dydave commented

That's settled: We're going to 2.x.

I've got all the necessary information.
No blocker at this point.

I'll update this issue with the new merge request for the correct branch, mostly likely later today ... I need to handle the security releases first 😅

dydave opened merge request !3

dydave’s picture

dydave commented
Status: Needs work » Needs review

OK, quick summary of this new version:

1 - Created branch 2.x:
The split was saved in the "backup" branch clone-11.x at #18 which was cloned to branch 2.x:
https://git.drupalcode.org/project/toolbar/-/tree/2.x

2 - Created in this issue fork a new feature branch based on 2.x: 3624202-clean-dev-release-2.x
https://git.drupalcode.org/issue/toolbar-3624202/-/tree/3624202-clean-de...

3 - Ported the changes from MR !1 and MR !2.

4 - Tested locally and fixed several issues on 11.4.

5 - Fixed all quality gates: cspell, nightwatch, phpcs, phpstan and phpunit.

The merge request !3 comes back all green 🟢, thus moving issue to Needs review:
See pipeline: https://git.drupalcode.org/project/toolbar/-/pipelines/974131

I tested successfully locally as well with:

  • Claro, Olivero themes
  • Admin Toolbar, ATools and ASearch
  • Shortcut, Announcement and other core modules
  • The Announcement feed tray displayed fine.
  • The vertical display seemed to work well too.
  • I also tested the responsive aspects (breakpoints), and such, everything worked fine, just as it does with the core module (D11.4).

 
Additionally, the fact that all the quality gates go through should also be a positive sign the changes in MR !3 should fix the stability of the module.
 
Once again, I would greatly appreciate to have your reviews and testing feedback on this updated version.
Thanks in advance!

smustgrave’s picture

smustgrave commented

1 small suggestion on the MR, not in front of my computer but was toolbar deprecated in 11.4 or 11.5? I can look up later also

dydave’s picture

dydave commented

Thanks a lot @smustgrave for taking the time to look at the MR, once again! 🙏

Removed in D12: https://www.drupal.org/node/3623310
Deprecated in 11.5.0: https://www.drupal.org/node/3616232

Support for 11.4.x is extra and "could" be considered "out-of-scope" if we're aiming at the very bare minimum.

Mostly, the extra changes to extend the support to 11.4.x are here:
https://git.drupalcode.org/project/toolbar/-/merge_requests/3/diffs#line...

Let me know what you think about the version we should support.

I'm more and more leaning towards doing the bare minimum: core_version_requirement: ^11.5
But since the code changes are not "too" complicated, all the tests currently pass on 11.4.x and the code is already there.... then we could also just keep it this way. 🤷‍♂️

smustgrave’s picture

smustgrave commented

So think we need to do 11.5 only because there may be changes in there that didn’t make 11.4 (haven’t looked) and we need to add 12 since users will have to download this when upgrading to 12.

Rest looks great!

dydave’s picture

dydave commented

I'm with you for 11.5.x: I thought about this as well yesterday and it would probably be better just to stick with what we know:
The code we have works with 11.5 out-of-the-box and we're not sure with 11.4.

For D12 @smustgrave, I gave you more details in Slack, not sure you were able to see my message:

Concerning support for D12:

@nicxvan made me doubt with his comment about backbone JS removed from D12...
https://www.drupal.org/project/toolbar/issues/3624031
Therefore, I thought: If the toolbar JS scripts require the backbone JS libraries and they're not there in D12, then we should probably not support D12 for this initial 2.0.0 version (?!)... since it is going to break for sure.
I found the History module: https://www.drupal.org/project/history
as an example and it shows >11.3
Which also comforted my opinion on maybe not supporting D12 for the first version.

I thought maybe we could try to push this one out quicker, then circle back and fix the D12 compatibility with the backbone JS issue and other changes, etc...
 

What do you think ?
Should we still allow installing the first version of the module on D12, even though we know it is going to break?

As soon as I can have your confirmation, I'll make the necessary changes so hopefully we could get the MR merged before the end of the day. 🤞

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Okay you are correct about the backbone but that means #3624031: Core no longer has the backbone and underscore libraries needs to be complete before a stable release can be made.

smustgrave’s picture

smustgrave commented

Can you do a 2.x dev release too when you get a chance so we can point tickets at 2.x-dev please

dydave’s picture

dydave commented

Thanks so much @smustgrave and sorry for the delay!

Thanks a lot for confirming the changes.
Everything is clear now .... I'll get the releases created ASAP.

smustgrave’s picture

smustgrave commented

Just fyi the backbone ticket must land too before a stable release. So unfortunately not just this ticket

dydave’s picture

dydave commented

OK, I added an extra commit to fix the Gitlab CI pipeline:

  • Skipped current nightwatch and phpunit jobs (11.4.x).
  • For the next minor nightwatch and phpunit jobs (11.5.x): Bumped 11.4-dev to 11.5-dev so the toolbar module could still be enabled with the requirement ^11.5 and run the tests.
  • Forced next minor jobs to run by default in MR and push, so we could get an automatic validation with tests.

 
At least we've got tests that could be manually triggered to ensure module's code stays compatible with 11.5.x (11.x).

Good news: The pipeline came back to green again 🟢

All the custom rules could be removed in the future, once 11.5.x is available and becomes the core supported version.

  • dydave committed 8a1c5ac9 on 2.x 
    task: #3624202 Added composer.json and Gitlab CI configuration. Updated...
dydave’s picture

dydave commented
Title: Prepare initial stable release 1.0.0 » Create an initial 2.x-dev release for module Toolbar
Status: Reviewed & tested by the community » Fixed

Let's move on to fixing the D12 JS related issues, there is going to be quite a lot of work there 😅

I merged the changes in 2.x, all the builds passed green 🟢
Created initial development release toolbar-2.x-dev so we could start tagging some issues.

I saw your work already in #3624031: Core no longer has the backbone and underscore libraries, which is definitely the next issue we should get in.

Thanks again for your help sorting out the branches and version requirements.🙏

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.