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.
Content added with drupal_add_region_content() does not show up in the region.
Comment | File | Size | Author |
---|---|---|---|
#35 | 713462-drupal-add-region-content-35.patch | 2.27 KB | Paul B |
#33 | 713462-drupal-add-region-content-test-33.patch | 1.79 KB | Paul B |
Comments
Comment #1
casey CreditAttribution: casey commentedRemoving drupal_add_region_content() and drupal_get_region_content() is a possibility, although I am not sure modules need to be able to add content to maintenance pages. This is currently only possible through drupal_add_region_content(), but I believe there are currently no possibilities for modules to do so.
Ideal IMHO would be if maintenance pages are build/rendered/alterable through drupal_render().
Comment #3
andypostI think this function inherited from D6 drupal_set_content() but limited by maintenance page for some reason.
For D7 we are using hook_page_alter() for purpose of adding some data to a region.
We should change doc block to point this or change behaviour to allow it work as D6 way.
Comment #4
andyposttagged
Comment #5
sonugoyalmca2006@yahoo.com CreditAttribution: sonugoyalmca2006@yahoo.com commentedi am using drupal D7, want to assign content in region using drupal_add_region_content, but content is not shoing in particular region. can you help?
Comment #6
q0rban CreditAttribution: q0rban commentedhook_page_alter() is not a replacement for drupal_set_content(). drupal_set_content() could be used inside of other functions, after certain data has been retrieved for instance. A way to achieve this in D7 with hook_page_build() would be to store that data in a static variable in the same way that drupal_add_region_content() is doing, and then retrieve it in hook_page_build(). Not an ideal workflow.
Comment #7
q0rban CreditAttribution: q0rban commentedComment #8
andypostAny reason to remove this?
1 days to next Drupal core point release.
Comment #9
q0rban CreditAttribution: q0rban commentedandypost said:
Actually, now that I'm looking at it, if you want to add content to a region, you should be using hook_page_build(), not hook_page_alter(). It's like the difference between hook_menu() and hook_menu_alter(). If you want to add new menu items, you use hook_menu(). If you want to tinker with existing ones, you use hook_menu_alter().
Comment #10
digitalsweetspot CreditAttribution: digitalsweetspot commentedI found a workaround to this issue for D7, and posted it here: http://api.drupal.org/comment/48353#comment-48353. Hopefully this helps out. I welcome any feedback.
Comment #11
David_Rothstein CreditAttribution: David_Rothstein commentedThanks for posting that, but this core issue needs to stay on Drupal 8 (backport to Drupal 7 is possible later, depending on what the fix is).
Comment #12
jwilson3Removing a feature from Drupal because its broken feels like the wrong thing to do.
* The amount of code to make this functionality actually work again is trivial (see the following patch below).
* The recommended workaround in #8 does *exactly* what this code already does in core, and makes modules/custom code more confusing (having to set custom variables and alter regions).
Here is a trivial patch that adds this functionality back to D7. The only thing left to do here would be
a) review the methodology suggested in this comment and patch
b) update this patch for D8
c) write a few tests that prove that you can add content to regions.
PS. I need this code because I need to pull content out of a panel region and place it into a theme region (a concept originally coined by Dave Reid for D6). It entirely depends on this function call, because inside a ctools panel layout definition, you don't have access to hook_page_alter() or hook_page_build(), and this function is really a very clean solution.
Comment #13
jwilson3aaaaand.... the patch. :/
Comment #15
mgifford#13: 713462-drupal-add-region-content-D7.patch queued for re-testing.
Comment #17
mgifford#13: 713462-drupal-add-region-content-D7.patch queued for re-testing.
Comment #19
jwilson3Rerolled for D8, so bot stops complaining.
Comment #20
andypostHow another piece of mark-up affects twig?
Comment #21
jwilson3Per #12.c
Comment #22
jwilson3I don't think this needs additional documentation, if the idea is to fix this functionality as it was working in D6. Also, tagging as Novice for test writing.
Comment #23
jwilson3I;ve updated the documentation for Assigning content to regions to reflect the fact that you need a patch from this issue to make it work in D7 and D8. After this gets in, we should remove that notice.
Comment #25
schrammos CreditAttribution: schrammos commentedwe are testing the patch
Comment #28
jwilson3Did this get anywhere in Amsterdam? Looks like it might need a reroll.
Comment #29
jwilson3Comment #30
dawehnerThis api does not exist anymore in d8.
Comment #31
mgiffordComment #32
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedRe-rolled patch #19 from recent 7.x branch.
Comment #33
Paul B CreditAttribution: Paul B commentedThe patch adds a failing test.
Comment #35
Paul B CreditAttribution: Paul B commentedThe patch from #32 with test
Comment #36
mgiffordWhat's the best way to test this? The issue summary hasn't been updated yet making it hard to RTBC.
The code looks good from a quick review. It applies of course.
Comment #37
Paul B CreditAttribution: Paul B commentedComment #38
jwilson3@#36:
Easiest way to test this would be to write code that adds some text to a region.
Eg, add
drupal_add_region_content('header', 'Welcome!');
inside theseven_preprocess_page(&$vars)
function.^ this is totally untested, so I'm not even 100% sure this will work as expected, but "in theory" it should work.
Comment #39
Paul B CreditAttribution: Paul B commentedWhy in seven_preprocess_page()? Is there a reason why you shouldn't call the function in a page callback?
Comment #40
jwilson3He asked for an example of how to test it. Sure, you can place the function call anywhere you want where it will run, however, #38 was the path of least coding to actually test the feature.
Comment #41
dcam CreditAttribution: dcam commentedThis looks ok to me and it does seem to be a regression from 6.x behavior. I haven't been able to determine whether that was intentional or not. Since there are reports of this being a regression I checked to see what was adding this content to the page in 6.x. theme_blocks() was doing it, along with adding the blocks to the page. It looks like this dual-responsibility was eliminated, but the additional content code was just deleted rather than moved elsewhere. I suspect this might have gotten lost in the shuffle of functions where theme_blocks() got changed to block_get_blocks_by_region() and some functionality got put into template_preprocess_page().
After finding all that out, I think template_preprocess_page() probably is the proper place to put the code, rather than adding it back in via one of Block's functions. It's probably not the best way to add random content to a region, but if we're keeping it in the 7.x API then it's true that this function doesn't do what it's supposed to do.
I tested the patch. Before applying it, I couldn't add content to a region with drupal_add_region_content(). After applying the patch the content appeared at the bottom of the region (which is where it would have appeared in 6.x). By the way, this wasn't testable by adding drupal_add_region_content() to seven_preprocess_page() as was suggested in #38. Maybe template_preprocess_page() had already run by that point. Instead I tested it by adding content to a taxonomy admin page callback.
There is a code formatting problem with the new test function, testDrupalAddRegionContent(), in theme.test. The entire function needs to be indented one level and there is an extra line after the function that should be deleted. If anyone is interested then they can roll a new patch with those changes. I'm going to go ahead and mark this RTBC, however, so that @David_Rothstein can go ahead and take a look at it. It's possible he can just go ahead and make those changes on commit.
Comment #44
dcam CreditAttribution: dcam commentedComment #45
jwilson3Thanks for RTBC!
Comment #46
David_Rothstein CreditAttribution: David_Rothstein commentedHm, this is kind of a mess, but yeah, the function still exists so making it work seems like the right thing to do :)
Committed to 7.x - thanks! I fixed the formatting issues noted by @dcam in #41 on commit.