Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
After #2445309: substr() expects parameter 1 to be string, object given in devel_silent I had a look more in depth at devel_silent()
. The current implementation of this function not work properly because the path returned by Url::fromRoute('')->toString()
(and the previous (implicit) behavior) has a leading slash and if your site runs in a subfolder the returned path is base_path + internal_path.
I think we need absolutely to fix this issue and modernize a bit devel_silent()
. Patch attached
Comment | File | Size | Author |
---|---|---|---|
#17 | interdiff-12-17.txt | 670 bytes | willzyx |
#17 | devel-devel_silent-2448391-17.patch | 11.56 KB | willzyx |
Comments
Comment #1
willzyx CreditAttribution: willzyx commentedComment #3
willzyx CreditAttribution: willzyx commentedoops.. rerolled
Comment #4
willzyx CreditAttribution: willzyx commentedBumping to major. A lot of devel's functionalities not work properly due to this issue
Comment #5
moshe weitzman CreditAttribution: moshe weitzman at Acquia commentedLooks good. In addition, you can exclude system.db_update route and remove the clause about update.php. update.php is no longer its own file in D8. Feel free to commit after that change.
Comment #6
willzyx CreditAttribution: willzyx commentedupdate.php seems to be a front controller after #2540416: Move update.php back to a front controller so i think we need that check
Comment #7
mermentau CreditAttribution: mermentau as a volunteer commentedThis is kinda urgent now due to changes in core that causes:
The patch fixes that.
Comment #8
mermentau CreditAttribution: mermentau as a volunteer commentedDisregard #7 as the error was caused by a corrupt download of Core.
Comment #9
moshe weitzman CreditAttribution: moshe weitzman at Acquia commentedComment #11
willzyx CreditAttribution: willzyx commentedMoshe I had not yet commited this because I think that is not the better solution. The current patch had some problems:
- It explicitly exclude some routes. From the viewpoint of maintainability it is not good.. we should constantly follow the core changes and keep our code aligned to HEAD.
- it not exclude non html responses, so we have not solved the problem. A lot of non html responses returned are still affected by this issue (a simple example: entity reference ajax call not work with this code). In
devel_shutdown_real()
we try to not break non-html pages relying ondrupal_get_http_header
that was removed by core.- it miss test coverage
Proposed solution:
1) Enable devel only on html pages
2) Find a mantainable and simple way to do this
3) Find a way for explicitly exclude some route from devel output (eg add a _devel_silent requirement to routes) so developers are able to exclude theirs routes
4) add test coverage
Comment #12
willzyx CreditAttribution: willzyx commentedHere a patch that address proposed solution in #11 :
1) Enable devel only on html pages by checking the response type; No more explicitly exclude some routes
2) Is possible to manually disable devel adding the '_devel_silent' requirement to route definitions
3) Updated the doc block of devel_silent() to make a little bit clear how it works
4) Added test coverage
Comment #14
willzyx CreditAttribution: willzyx commentedGET /checkout/core/install.php returned 0 (0 bytes).
mmm.. retesting
Comment #17
willzyx CreditAttribution: willzyx commentedSeems that sending a request with
User-Agent: ApacheBench/1.0
header break the bot.. intrestingFor now I commented that assertion
Comment #18
moshe weitzman CreditAttribution: moshe weitzman at Acquia commentedLooks terrific - thanks. Sorry for my premature commit.
Comment #19
willzyx CreditAttribution: willzyx commentedIt's my fault. I would have to change the status of the issue and explain my reasons
Thanks for the patience :)
Committed and pushed to 8.x!