Exclude random token string in link from Admin Toolbar Search

Thanks @ressa for creating this issue, once again! 👍

Very good catch, not a very obvious one 😅
Really not sure how you could have stumbled on this one 😆
Because these strings are very random and change all the time!

It would be great if we could get all the refactoring tickets for the Automated Tests rolled in the module before moving forward with this one....
We would most likely need to add a check for this in #3550652: Automated tests: Simplify admin_toolbar_search FunctionalJavascript tests and more specifically here:
https://git.drupalcode.org/project/admin_toolbar/-/blob/22bc71e22a155de7...

• Get first 5 characters of the generated token from the HTML markup in the toolbar.
• Submit it in the search as a query and assert suggestions are empty or do not contain the URL of the flush link.

 
I think it's not the only one: The logout link is the same ....
(try typing logout in the search, check the query string after the question mark ?, type in the search a few letters of the query string and you'll see the same behavior as described in the IS for the flush link ==> Logout will come out in search suggestions)

So we would most likely need to do a CSS query search (with an elementExists CSS search query) to select all links in the toolbar admin menu with a query string and loop through selected links with the same steps as above (get start of query string, submit in search and confirm URL is not found in suggestions).
This would make it bullet proof 👍

In terms of the JS changes in the search to effectively fix this, we would have to change a bit the JS array:
Drupal.behaviors.adminToolbarSearch.links
Loaded when the search field is focused with function populateLinks:
https://git.drupalcode.org/project/admin_toolbar/-/blob/3.6.2/admin_tool...
Break the URL on the question mark and keep the first element in:
'label': label + ' ' + this.href, (replace the this.href with the stripped off URL)
(==> strip off query string from URL and save in labels loaded array of links)

Pretty straight forward, I "think". 🙂
 

I would be very happy to fix this ASAP, but ideally, if possible, it would be great if we could first fix the refactoring tickets...

In particular, on top of the ones already in the stack (9 issues ready to go 🥳), I've got a major refactoring lined up for the file admin_toolbar_search.js with mostly Vanilla JS, code clean-up and ESLINT fixes 🤩
(Not pushed yet in merge requests.... I think I've flooded enough the queue 😅)

It would be great if we could add this issue's fix right after, plus I've got #3532958: Make obvious when toolbar search returning no results almost ready as well, with the refactored file 👌
(Same thing ==> we need the tests before proceeding with this....)

After refactoring the Automated Tests, I'm now super familiar with module's code base ... I pretty much know it by heart now 😆
I could rewrite the module from memory 😆
It's like reading the same book a hundred times 😆

I'm trying to work on this module with the community (== you pretty much 😅, my contribution partner 🙏), as much as possible, but if I see the refactoring is holding up tickets for too long, I'll most likely move forward and get pending merge requests rolled-in ASAP.
I'm the maintainer after all and thus, have all the required rights and permissions to proceed with these changes, as long as I am confident and ready to take full responsibility 👍
As soon as the refactored tests are in place, I am super confident taking responsibility with any changes.

I've updated the list in the issue summary of the Meta Roadmap ticket with the 9 tickets ready to go at the top 🥳
If there is any ticket, merge request or change you're not sure about, please let me know and I'll most likely get them merged straight away ... Because I have great doubts anybody is really going to help or answer, other than you 😅 and I'm not planning on waiting for months to get feedback or reviews on these tickets.... We've still got so much to do....

As always, your feedback is more than welcome, if you disagree with some of the suggested changes or have different ideas, vision, etc... or if you catch anything I could have missed, some untranslated labels, textual improvements, etc... as you did many times before: It would be super helpful!
It has been so constructive working with you on previous issues, in particular, I'll always remember your help bringing me down to earth when I got carried away on the hoverIntent ticket (with the useless config options) 😆

In short, if anything is beyond your technical skills or understanding, let me know and I'll take care of it and most likely get it merged ASAP 👌

Always a pleasure working with your @ressa! 🙂
(missing our back and forths on the scroll up/down ticket 😆)
Thank you very much, once again for your precious help! 🙏

Thanks again @ressa for the great help documenting and describing this issue, as usual! 🙏

Funny thing, but the solution described above at #2 is not all the one that was implemented in the end 😅
The reason the solution above was not satisfying is because we could not just save menu links URLs without the query string, since it would be lost when building search results suggestions when the real links would need to be displayed, for example, the logout link, etc...
Therefore, the query string needs to be striped off when the search is performed at run-time (in function handleAutocomplete ).

Quick update on this issue following the latest code changes:
This issue should have been fixed in related issue #3564229: Admin Toolbar Search: Refactor admin_toolbar_search.js, in particular, with commit:
https://git.drupalcode.org/project/admin_toolbar/-/commit/40fc5b84a8aad7...
with the following piece of code:
https://git.drupalcode.org/project/admin_toolbar/-/blob/40fc5b84a8aad768...

              // Strip query strings from link URLs for searching to prevent generated
              // tokens or destinations from appearing in the search results.
              const linkUrl = element.linkUrl.split('?')[0].toLowerCase();

Additionally, the automated tests related to this feature have been added to related:
#3550652: Automated tests: Simplify admin_toolbar_search FunctionalJavascript tests, in particular, with commit:
https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/175/di...

Since I remember you already tested this and confirmed it worked as expected, in the JS refactoring issue and that some tests coverage should be put in place for this piece of logic, we should probably be able to consider this issue as Fixed, for now.

This comment should document all the work related with this issue and allow us to circle back on the corresponding code changes if needed in the future.

Feel free to let us know if you spot anything else that we could have missed in this issue, or create a new one at any time, we would certainly be glad to take another look. 👍

Thanks again for all your great help @ressa!
Great catch, once again, really not an easy one! 🥳👍