[regression] Uncaught TypeError: Cannot read properties of null (reading 'style') (toolbar.js)

I ran into this same issue! The patch as-is didn't quite work, but not because the code itself wasn't sound. It seems the patch formatting itself had an issue? Here's the patch I used (the code was borrowed @wendell in the original patch). Note that I was able to reproduce this when adding an inline block via Layout Builder.

Can we get a test showing the issue?

I encountered a similar issue while using the Gin Toolbar module. This module modifies the toolbar's ID to gin-toolbar-bar, making it untargetable in toolbar.js.

I am attaching a video recording of the behaviour that I am encountering.

This allso happens with Bootstrap's modal dialogs

The patch fixed the Ajax error on Drupal 10.2, the Gin theme 8.x-3.0-rc8 and the Gin Layout Builder 1.0.0-rc5.
As mentioned above the Gin Layout Builder modified the ID for the secondary toolbar to "gin-secondary-toolbar" and the code from the toolbar.js fails to find the element in the DOM.
It is always safer to check if the element exists before applying a style.
We will use the patch from #2 to fix the issue.

Patch #2 fixes issue for me with Drupal 10.2.1, the Gin theme 8.x-3.0-rc8 and the Gin Layout Builder 1.0.0-rc5.

Thank you.

#2 works for me with Drupal 10.2.2 and Gin 8.x-3.0-rc7. Thanks.

A maintainer asked for tests.

The test is hard to make because it happens only with the conjunction of gin/gin toolbar because gin removes some classes and toolbar.js does not have a proper check whether element exists (which should be a good practice anyways). Gin Toolbar has 30k+ installs, pretty big impact imho

#2 also resolves this with Gin and Layout Builder. I'm bumping to Major is it makes LB un-usable. A test may be tricky since this seems to only be surfaced by Contrib modules...

Running into this in putting a Drupal.dialog (not ajax) in a contrib module.

This patch fixes it for me, but the fact that adding a dialog in my module triggered this problem in toolbar makes me wonder if i am not doing something to scope my dialog correctly— or if dialog has a bigger bug?

My error was not mentioned toolbarBar is null, but search still worked fine to find this issue.

Patch fixed the error after clearing Drupal and Browser caches.

Interesting fact: custom link dialog ( https://www.drupal.org/docs/develop/drupal-apis/ajax-api/ajax-dialog-boxes → ) + bootstrap theme + css + js keeps braking (in multiple ways per upgrade) every single time I update Drupal for years.

I will try to test if this is the case for this issue too when I get some time: https://www.drupal.org/project/ajax_comments/issues/3412943 🐛 Delete comment modal dialog broken with Drupal 10.2.x Active

It is also custom link dialog + bootstrap theme + css + js

Have the same issue as #14 and the patch solves the issue. Thanks!
We need to figure out how to have tests for this.

I can confirm patch #2 fixes https://www.drupal.org/project/ajax_comments/issues/3412943 🐛 Delete comment modal dialog broken with Drupal 10.2.x Active , however, the look (CSS styling) of the popup dialog is still broken

I think the issue is deeper as this patch only fixes the consequence, but not the reason why styles are not properly passed to AJAX dialogs/popups

I would just like to raise the question that this issue is marked as Version: 11.x-dev but that it is definitely an issue for Drupal 10.2.3 too.
I think my statement is backed up by this issue 🐛 Drupal ajax error after D10 update from version 10.0.11 to 10.2.1 Needs review .

Thanks all, if I'm wrong then please someone put me right!

It seems multiple modules as well as the core itself were affected by some change in 10.2 that caused this. This issue prevents many websites from updating to 10.2, while 10.1 EOL is getting closer and closer (17. 6. 2024)

I just successfully tested https://www.drupal.org/files/issues/2023-12-20/fix-toolbarjs-null-handli... → on 10.2.4

I've just created MR with implemented @wendell and @malik.kotob work and additional check.

Here's also patch for this

Is this needs work, or needs review?

The last patch seems to fix the ajax part of the problem I had

Following 🌱 [policy, no patch] Better scoping for bug fix test coverage RTBC I don't think tests are required here.

The issue has clear steps to reproduce, and the fix is trivial - just a guard against NULL. It has already been verified with manual testing by people in this issue. It is self-contained to the toolbar JS, doesn't require any new code, a regression will only leave us back where we started, and adding test coverage is tricky without jumping through a lot of hoops.

The patch looks good to me, marking NR for someone to confirm I have applied the new testing gate correctly above.

A b/c break was inadvertently added by https://git.drupalcode.org/project/drupal/-/commit/6a43b511df9e26aa5abf0... when jquery was swapped out for vanilla JS.

Previously, jQuery was more tolerant of an element not found condition. The vanilla JS results in an uncaught exception.

I was going to remove the needs tests tag, but Dave beat me to it :D

Oh, if the break was in that issue then I'm even more confident from reading the diff there. Happy to mark this RTBC and let another committer confirm.

Also adding the regression title tag given that this used to work and we broke it in 10.2.

Committed 41943e8 and pushed to 11.x. Thanks!

nod_ → closed merge request !7333

This is eligible for a bug fix release in 10.2.x, cherry-picked it back there as well.