Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Files that need converting are:
- core/modules/toolbar/toolbar.module
Comment | File | Size | Author |
---|---|---|---|
#12 | 1999442-toolbar-request-12.patch | 606 bytes | kim.pepper |
#5 | 1999442-toolbar-request-5.patch | 627 bytes | kim.pepper |
#1 | 1999442_1-remove-GET-variable-toolbar.patch | 588 bytes | chertzog |
Comments
Comment #1
chertzogPatch attached.
Comment #2
chertzogComment #3
kim.pepperLooks good to me. Thanks chertzog!
Comment #4
alexpottNeeds a reroll... and we already have the request object so this patch is even simpler...
Comment #5
kim.pepperUses existing request object as per #4.
Comment #6
Crell CreditAttribution: Crell commentedI don't know what this code is doing, but modifying the request is the correct action in about 0.00053% of cases. And those are Drupal design flaws we haven't fixed yet. The odds of this being one of them are low. :-) What's going on in this module, and can we change it to not be wrong?
Comment #7
Crell CreditAttribution: Crell commentedBumping. I'm especially confused by the use of $_GET['q'], since that's long since been eliminated as a path value.
Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedthis is either cruft and can be safely removed, or it hides a bug..$_GET['q'] is definitely not working..i guess some input from a maintainer of toolbar module is needed
Comment #9
kim.pepperFrom MAINTAINERS.txt:
Comment #10
kim.pepper#5: 1999442-toolbar-request-5.patch queued for re-testing.
Comment #11
Crell CreditAttribution: Crell commentedOh good.
Let's try removing it entirely and see what testbot says breaks. :-)
Comment #12
kim.pepperRemoved reference to $_GET as per #11. Seeing what the bot thinks.
Comment #13
Crell CreditAttribution: Crell commentedOK then! :-)
Comment #14
YesCT CreditAttribution: YesCT commentedThis issue was RTBC and passing tests on July 1, the beginning of API freeze.
Comment #15
alexpottI've manually tested the toolbar with page caching enabled and everything is still working as expected.
Committed 7617975 and pushed to 8.x. Thanks!