I am running into an issue where when I have login_destination and admin_toolbar_tools enabled that I lose the dropdown styling and functionality. Is anyone else running into the same? I will be doing some debugging and will hopefully come up with more information later today.

Comments

Christopher Riley created an issue. See original summary.

Christopher Riley’s picture

It appears that when login_destination is enabled along with admin_toolbar_tools that the following is getting dropped:

<button class="toolbar-icon toolbar-handle"><span class="action">Extend</span><span class="label">Content</span></button>

Suggestions?

darkstartom’s picture

I am also having a problem with admin_toolbar when used with login_destination. When the screen size is reduced to the point where the menu moves to the left of the content I get an Ajax error in the console and I lose menu functionality.

AjaxError:
An AJAX HTTP error occurred.
HTTP Result Code: 403
Debugging information follows.
Path: /toolbar/subtrees/0bc1tVE8yBYygIeXLYCwuhreTRJ8dad54ujpjSlDdgk
StatusText: Forbidden
ResponseText: {"message":""}

I'm using login_destination 8.x-1.x-dev with admin_toolbar 8.x-1.23 and Drupal 8.5.1.

Any suggestion as to how to fix this? Thanks!

darkstartom’s picture

Upgrading to the development version of admin_toolbar (dated 20 Mar 2018 at 16:58 UTC) seems to have fixed the problem.

Christopher Riley’s picture

Ok I will check into that thanks.

tclark62’s picture

I can confirm that using this module in conjunction with Admin toolbar still causes a conflict. When I uninstall Login Destination, I no longer receive this Ajax error and the vertical toolbar functions properly. I'm using the most recent version of Admin toolbar (8.x-1.26).

megachriz’s picture

This issue could be a duplicate of #2947616: SetOptions on null causing error which also fixes issues when this module is used together with admin_toolbar_tools.

miroslav-lee’s picture

I confirm that. I applied the https://www.drupal.org/project/login_destination/issues/2947616#comment-... patch and I lost this problem.

justcaldwell’s picture

In case anyone else is still have this issue, just noting that the patch from #2947616: SetOptions on null causing error did not solve the problem for us. I previously applied the patch to no effect, and have now updated to alpha1 (which includes that fix) -- no luck.

megachriz’s picture

@justcaldwell
Alright, then this issue is not a duplicate (which I suggested in #7). Can you provide the steps to reproduce the bug? I'm not facing this issue on sites where I have installed Login Destination on, despite that on all of them admin_toolbar_tools is installed.

justcaldwell’s picture

@MegaChriz
Sure, we're on Drupal 8.6.10, Admin Toolbar 1.26 and Login Destination 1.0-alpha1. With admin_toolbar_tools and login_destination enabled, if the toolbar is in vertical (small screen) mode, the there are no dropdown arrows, the dropdowns do not function and the ajax error in #3 appears in the console.

If I disable either admin_toolbar_tools or login_destination, everything works as expected and the ajax error disappears.

At this point, I'm guessing there's some other unknown at work in our instance. I'll try to set up a clean install in the next few days to see if I can replicate it there.

megachriz’s picture

@justcaldwell
Guess the issue does appear on at least one of the sites I've installed admin_toolbar_tools and login_destination. I hadn't noticed the arrows were missing in the vertical toolbar. Everything appeared to work correctly, but I now see the AJAX error in the console as well. I mistakenly assumed the error would cause more issues, like unresponding menu items for example (that's how interpreted #3's "I lose menu functionality").
Also, when I resize the window to a bigger format (so that the toolbar gets horizontal again), the dropdowns on the horizontal menu keep working. So the issue is only limited to the vertical toolbar, I guess.

justcaldwell’s picture

Thanks for the confirmation, @MegaChriz. I'm no closer to a resolution, but I was able to reproduce the issue in a clean instance of Drupal 8.6.10 with Admin Toolbar 1.26 and Login Destination 1.0-alpha2.

To be clear, the toolbar works as expected at all times when it is in horizontal/top orientation -- the error only occurs when it is in vertical/side orientation. Specifically, the problem is:

  • There is no sub-menu dropdown indicator/arrow on any of the menu items (e.g. Content, Structure, Appearance, etc.), and the sub-menu items are not available
  • The ajax error described in comment #3 appears in the console.

I tested both with and without admin_toolbar_links_access_filter enabled with the same result. I took a quick screencast of the issue:

Also, adding the related issue from the Admin Toolbar queue.

rsvelko’s picture

I'll be watching this one now. Pretty strange story so far in this issue... At some point I'll join the debugging party.

justcaldwell’s picture

After digging around in the core toolbar issue queue, I think I've identified the problem. I hope someone can offer a solution. The key factors are:

  1. The admin toolbar subtrees (menu links) are re-loaded via ajax only when the toolbar goes into vertical orientation (that happens when the client width is narrow enough, or the user clicks the orientation-change button).
  2. When a request for subtrees is received, the core toolbar module checks a hash of the toolbar subtrees against the hash it expects, and denies access if they don't match (see checkSubTreeAccess() in /core/modules/toolbar/src/Controller/ToolbarController.php).
  3. The login_destination.module implements hook_link_alter() to append the '?current=[current_path]' param to the user.login and user.logout routes.

The changed link urls (3) result in a different hash being sent with the ajax call than the one expected by the toolbar module (2), hence the 403 Forbidden error that shows up in the console, but only when the toolbar changes to vertical mode (1).

The quickest way to verify all this is to comment out login_destination_link_alter() and clear caches. The toolbar should immediately start working as expected in vertical mode.

Edit: changed item 3 above. The 'current' param is set to current path, not the user profile path.

rsvelko’s picture

Great work! The actual fix should now be the easy part.
My intuition tells me the fix should happen on admin_toolbar_tools territory=codebase.

justcaldwell’s picture

Thanks, @rsvelko. I think I now understand what's happening.

  • As mentioned above, login_destination implements hook_link_alter() to add the current path to user.login and user.logout routes.
  • admin_toolbar_tools adds a (redundant) Logout link to it's 'tools' menu, which 'resets' the subtree hash value (I say redundant, because there's already a Logout link under the user menu in the default toolbar.
  • login_destination_link_alter() immediately re-writes that extra logout link to add the current path param, which will result in a different hash if/when the subtrees are requested via ajax.

So, I can see a couple possible approaches:

  1. Ask admin_toolbar_tools to remove the 'redundant' Logout link, or
  2. Update that extra logout link from login_destination (similar to what login destination currently does to to deal with the default toolbar's user links.
justcaldwell’s picture

Status: Active » Needs review
StatusFileSize
new736 bytes

I tried to disable caching of the admin_toolbar_tools links with no luck, so I went the route of removing the Logout link entirely. Patch attached.

rsvelko’s picture

- removing that links won't be harmful to anything/anyone I guess?
- did not test it yet, but patch seems OK on 1st sight.

justcaldwell’s picture

Removing the link is definitely open to discussion. It doesn't break any functionality, but someone used to logging out that way could certainly miss it. It's just the only way I've found so far to solve the problem.

The log out link under the top-level user toolbar item is still there, so I guess I consider the extra link a convenience. I'd rather sacrifice it than the sub-menu functionality when in horizontal mode.

metalbote’s picture

Wouldn't it make more sense to replace the hook_link_alter with an EventSubscriber and create a redirect for logout route?

bgreco’s picture

Updated patch #18 to be compatible with the current version of admin_toolbar_tools.

gaurav.kapoor’s picture

Status: Needs review » Needs work

I am currently using Drupal 9.2.7, admin_toolbar 3.0.3, and 8.x-2.x version of login destination. I was able to reproduce the issue in my local, we do get a one-time AJAX error in the console which goes away on refresh. Also, after applying the patch, the admin toolbar still not working when using the vertical mode of the toolbar.

rsvelko’s picture

watching this one.

gaurav.kapoor’s picture

Status: Needs work » Reviewed & tested by the community

I must have missed clearing cache earlier after applying the patch, the patch is working fine and ensures that module doesn't cause any problems when using with admin toolbar. Hiding the links shouldn't be a problem as developers can provide logout link anywhere and ensure users are able to logout easily.

I am not sure if event subscriber suggested in #21 would be a better solution, but the patch provided in this issue does solve our problem.

leisurman’s picture

Patch #22 works on Drupal 9.3.19

rsvelko’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed patch #22.

  • rsvelko committed 2092dad on 8.x-2.x
    Issue #2951265 by justcaldwell, bgreco, rsvelko, Christopher Riley,...
rwam’s picture

Status: Fixed » Needs work
Related issues: +#3313309: Re-add Logout link for those who want

Cool, update installed and our customers are missing the logout button. So maybe is it possible to rethink this solution? See also https://www.drupal.org/project/admin_toolbar/issues/3313309

BTW: we and all of our customers are using horizontal toolbar and nobody didn't recognize any issues. But: updated yesterday and today we have the first support requests about the missing link. Annoying.

rsvelko’s picture

@rwam
For the moment I'd propose you go and manually comment out/delete the function login_destination_menu_links_discovered_alter() located on line 129 of the file: login_destination.module

This will bring back the logout link.

rwam’s picture

Hi @rsvelko,
thanks for your fast response. I have already used it as a patch locally and rollout the change to get links working again. If I found some time I'll try to find another solution for the original issue.

sputniktea’s picture

StatusFileSize
new709 bytes
new709 bytes

The patch in #22 did not work for us on drupal 10.1.x with php 8.1.
Therefore, we created a patch that removes the lines creating the login button in admin_toolbar.

We are not really happy with our solution, maybe there is a way to fix the hash creating mechanism in admin_toolbar 🤷‍♂️

rwam’s picture

Updated patch to work with current 8.x-2.x