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.
Updated: Comment #0
Problem/Motivation
- Following #2062439: Provide listing of custom block entities and #2058321: Move the 'place block' UI into the block listing, there are two entry points for creating a new custom block: The custom block listing at
admin/structure/custom-blocks
, and the block placement form atadmin/structure/block
. - When you add a custom block from
admin/structure/block
, submitting the custom block creation form takes you directly into the "place block" form for an instance of this new block, and then subsequently redidirects back to the place block form. - When you add a custom block from
admin/structure/custom-blocks
, the behavior changes based on whether you have only one custom block type:- If you have only one custom block type, you are redirected back to
admin/structure/custom-blocks
, as you'd expect. - If you have multiple custom block types, you first select your block type, then fill out the custom block form... and then are unexpectedly redirected into the place block workflow and then to
admin/structure/block
, despite that you might not have any intention of placing the block in the first place.
- If you have only one custom block type, you are redirected back to
Video illustrating this adventure:
https://www.dropbox.com/s/b15zhahytrww9sn/custom_block_redirect.mp4
Proposed resolution
Always redirect back to admin/structure/block/custom-blocks
if that's where you start.
Additionally, if you started on a theme-specific page like admin/structure/block/list/seven
, you will be redirected there.
Remaining tasks
Remove the seven theme override of the custom block list theming (ie this works in stark)
User interface changes
The user is consistently redirected to admin/structure/custom-blocks
if that's the starting point.
API changes
Probably none.
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#24 | custom-blocks-2068053-24.patch | 9.66 KB | tim.plunkett |
#22 | custom-blocks-2068053-22.patch | 9.65 KB | tim.plunkett |
#20 | interdiff.txt | 10.68 KB | tim.plunkett |
#20 | custom-blocks-2068053-20-FAIL.patch | 5.34 KB | tim.plunkett |
#20 | custom-blocks-2068053-20-PASS.patch | 10.61 KB | tim.plunkett |
Comments
Comment #1
larowlanThis was fixed in #2064591: Missing test coverage for multiple custom block types
Comment #2
tim.plunkettApparently it wasn't, according to xjm.
Comment #3
tim.plunkettComment #4
larowlanGrr you can see the destination in the URL on the list screen but either it's not in the links or overlay stuffs it. Hoping it's the latter cause the tests do exactly what the video does. The only difference is overlay.
Comment #5
larowlanNope doesn't work. Has to be seven.
Comment #6
larowlanYep http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/core/theme... missed the override. Mystery solved.
Comment #7
tim.plunkettI'm not sure which is stranger, seven_custom_block_add_list completely bypassing the preprocess, or custom-block-add-list.html.twig using the "node-type-list" class :)
Good find!
Comment #8
larowlanComment #9
larowlanSo in terms of 'needs tests' do we need to duplicate the tests with the seven theme enabled
</groan>
Comment #10
tim.plunkettCan't we just gut/remove the seven override? Also, put the test into a helper method and run it twice...
Comment #11
andymartha CreditAttribution: andymartha commentedI can't comment on tim.plunkett's idea in #10, but after applying the patch in #8 by larowlan, I can confirm that desired redirect effect happens.
Here is a video of after the patch: http://www.youtube.com/watch?v=8PaGHF110Pg
Comment #12
tim.plunkettThis still needs tests.
Comment #13
larowlanAgreed approach: remove the override in seven
Comment #14
tim.plunkettWorking on this.
Comment #15
tim.plunkettWell this is fun!
CustomBlockInterface::getTheme() and CustomBlockInterface::setTheme() are never actually used.
Or, that is, setTheme() is never used.
So this isn't even really related to the OP, but is covered under the title.
Comment #17
tim.plunkettCurrent URL is 'http://ns4007744.ip-192-95-30.net/checkout/admin/structure/block/add/custom_block%3A35922bf3-3d37-4bed-8502-2a192c94b332/seven'.
Current URL is 'http://ns4007744.ip-192-95-30.net/checkout/admin/structure/block/add/custom_block%3Ae570ce8e-e2b9-4227-9105-5d711f7568ce/bartik'.
Those are wrong because it always 'stark' due to getTheme() always returning NULL because setTheme() is never called:
CustomBlockFormController:
Comment #18
tim.plunkettIf we try to do something like
block/add?theme=bartik
, that breaks drupal_get_desintation (which is insane, btw).So I don't have any obvious solutions on how to track the theme name from page to page...
Comment #19
larowlanWe discussed this in irc and decided not to use drupal_get_destination
Comment #20
tim.plunkettOkay, here we go!
We can't use a proper local action because we need a query string, so we fake it.
CustomBlockController::addForm() now checks $request->query
seven_custom_block_add_list() is not outright removed yet, but it is brought into line with template_preprocess_custom_block_add_list().
Comment #21
tim.plunkettBy the way, I have this hunk in about 3 different issues, I've been doing a lot with query strings lately :) It's in #2083877: Add a theme selector to the place block form which is RTBC
Comment #22
tim.plunkettThat went in.
Comment #23
larowlanThanks @timplunkett for resolving this
Comment #24
tim.plunkett#2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests is going to break everything.
Comment #24.0
tim.plunkettUpdated issue summary.
Comment #25
webchickCommitted and pushed to 8.x. Thanks!
Comment #26
tim.plunkettThank you!
Comment #27.0
(not verified) CreditAttribution: commentedUpdated
Comment #28
DarkteK CreditAttribution: DarkteK commentedIf anyone is having this issue in Drupal 9, you can do this:
Comment #29
smulvih2I still get this inconsistent behavior in 9.5.5. Adding a submit handler like in comment #28 worked for me.