The $toolbar variable in toolbar.js is set to an empty jQuery object after an AJAX request [#1858576]

The $toolbar variable is declared outside the scope of Drupal.behaviors.toolbar. The value is assigned in this line of code.

$toolbar = $(context).find('#toolbar-administration').once('toolbar');

On the first pass through attaching behaviors, $toolbar gets assigned a jQuery object with one element. On any subsequent pass on the save page, such as after an Ajax request, $toolbar is set to an empty jQuery object because the .once() method will on select anything on a second pass.

Screenshot the Chrome development tools showing the $toolbar variable is assigned an empty array after an AJAX call on the page

We need to update the code so that $toolbar is assigned only on the first pass through the attaching of behaviors.

Comment	File	Size	Author
#13	toolbar-1858576-13.patch	942 bytes	David_Rothstein
#10	1858576_toolbar-var_10.patch	1.43 KB	jessebeach
#7	toolbar-1858576-7.patch	941 bytes	David_Rothstein
#4	toolbar-1858576-4.patch	1.41 KB	jessebeach
#1	toolbar-1858576-1.patch	827 bytes	David_Rothstein
	toolbar-variable-reset-on-ajax-request.png	93.1 KB	jessebeach

Comments

Comment #1

David_Rothstein commented 24 December 2012 at 23:29

Priority:	Normal	» Critical
Status:	Active	» Needs review

Status	File	Size
new	toolbar-1858576-1.patch	827 bytes

Marking #1872274: Editing breaks the toolbar as duplicate and moving to critical, since this really causes the toolbar to break.

Simple way to reproduce is to go to admin/structure/views/add and change the view from Content to Users - that kicks off an Ajax request which triggers this.

Here's a patch along the lines of above. Somehow the whole code flow seems a little odd, but I guess it's OK (since $toolbar is a global variable we must be assuming that there is only one of them on the page, but we still look for it again every Ajax request on the off chance that the Ajax request caused the existing toolbar to be destroyed and a new one to be added?)

Comment #2

David_Rothstein commented 24 December 2012 at 23:30

Component:

other

» toolbar.module

Comment #3

damien tournoud commented 25 December 2012 at 04:14

Status:

Needs review

» Reviewed & tested by the community

The patch makes sense. Let's get this in.

Comment #4

jessebeach commented 26 December 2012 at 15:29

Status	File	Size
new	toolbar-1858576-4.patch	1.41 KB

I'd like to change the wording just a tad in the comment in this patch. In this case $toolbar is not a global variable; it is referenced by the closure of the function Drupal.behaviors.toolbar.attach and $toolbar is scoped to the anonymous immediately invoked function expression that surrounds all of the code in this file. It's true that this code assumes one toolbar on the page, which is why it is selected by an id in $(context).find('#toolbar-administration').once('toolbar'), not a class. This patch doesn't change the behavior of the fix, just the name of the $newToolbar variable to $onceCheck and adjusts the comment accordingly.

Comment #5

webchick

she/they

English

Vancouver 🇨🇦

commented 26 December 2012 at 17:01

Status:

Reviewed & tested by the community

» Fixed

Committed and pushed to 8.x. Thanks!

Comment #6

David_Rothstein commented 27 December 2012 at 23:02

Priority:	Critical	» Normal
Status:	Fixed	» Needs work

Can we go back to something more like the original variable name? The thing that actually gets stored in this variable is a toolbar object, so calling it "onceCheck" doesn't sound right. I mean, lines like this just look really odd to me:

$toolbar = $onceCheck;

You're right about the code comment, though. It isn't a global variable (I forgot about the mega-function that wraps everything in Drupal JavaScript).

Comment #7

David_Rothstein commented 27 December 2012 at 23:09

Status:

Needs work

» Needs review

Status	File	Size
new	toolbar-1858576-7.patch	941 bytes

Something like this, perhaps. I couldn't come up with anything better than $newToolbar, but pretty much anything that has the word "toolbar" in it works for me.