I installed GMap on a site using a Fusion theme and GMap complained that it was being asked to create duplicate maps. It seems the Fusion call to block_list in fusion_core_grid_info() in template.php was causing all active blocks to be rerendered if not cached. Not sure if this is as intended but seems a waste of processing just to get a block count.

CommentFileSizeAuthor
#26 fusion-blocks-render-twice-1172280-26.patch961 bytesPoieo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rickmanelius’s picture

Subscribing

paskainos’s picture

+1

coltrane’s picture

So it's just the fact that blocks get rendered twice (and GMap doesn't like that) due to the calls to block_list() for every visible region?

aquariumtap’s picture

Status: Active » Postponed (maintainer needs more info)

I haven't been able to recreate this bug. Here's what I did:

  • Enabled Gmap, Gmap Location, Location, User Locations
  • Provided address, state and city fields for users through configuration options in admin/config/people/accounts
  • Created a block via Views that displays a map indicating the user's location
  • Displayed that block via Context on all user profiles

The map is displaying the correct location and I'm not receiving any errors. I also tried loading the block via admin/structure/block instead of via a Context module reaction, but still no error.

Are you still running into this issue? If so, could you provide more details on how you are creating the map block, and how you are displaying that block (via Context, via admin/structure/block, etc)

esmerel’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)
girishmuraly’s picture

Priority: Normal » Major
Status: Closed (cannot reproduce) » Active

I am experiencing the same issue. Its not related to GMap though.

I am using Contexts and Fusion_core on my site.

Every block was getting rendered twice and using debug_backtrace() I found that was due to the following call routes (snipped)

1. drupal_render_page -> context_page_build
2. drupal_render_page -> drupal_render -> theme('page') -> fusion_core_preprocess_page -> fusion_core_grid_info -> fusion_core_block_list -> block_list

No 1. seems sensible.

However, 2. seems too much to do. block_list() seems to be used to just see if blocks exist for a region and if so, set a width.
E.g. from fusion_core_grid_info()

$grid['sidebar_first_width'] = (fusion_core_block_list('sidebar_first')) ? theme_get_setting('sidebar_first_width') : 0;
$grid['sidebar_second_width'] = (fusion_core_block_list('sidebar_second')) ? theme_get_setting('sidebar_second_width') : 0;

The fact that it renders blocks too seems to have been overlooked. This seems to be a major performance drain.

sheena_d’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

No further development will be pursued in the 7.x-1.x branch of Fusion, so moving this to the 2.x branch.

aquariumtap’s picture

Status: Active » Postponed (maintainer needs more info)

Yes, Fusion checks to see if there will be any blocks populating a given sidebar. If not, the sidebar will not displayed, and the adjacent regions will be allocated their grid units.

Blocks need to be rendered at some point or another. There's no double render. Both the block module's block_list() and the context module's context_reaction_block::block_list() have static caching.

If a block seems to be rendered twice, it would have to be because both the block module and context module are rendering the same block. Do you have the block module enabled? Are you using it to place blocks into regions?

I haven't been able to reproduce this bug.

girishmuraly’s picture

@aquariumtap, thanks for the info. Here's my setup:

* block module is enabled, however it is not being used to place block into regions. It is enabled because a few modules (like AddThis) needs it as a dependency
* context module is enabled and used to place blocks into regions

The blocks are getting rendered twice.

Here is the simplified backtrace with function-name (and args as needed) in reverse order of invocation, when the block is rendered by context on the page. Note that it goes through context_page_build.

1. module_invoke
    Array
    (
        [0] => mycustom_module
        [1] => block_view
        [2] => custom_block_0
    )
2. _block_render_blocks
3. block_list
4. block_get_blocks_by_region
5. execute
6. context_page_build
7. drupal_render_page
8. drupal_deliver_html_page
9. drupal_deliver_page
10. menu_execute_active_handler

And here is the same block being rendered on the same page again, this time by fusion:

1. module_invoke
    Array
    (
        [0] => mycustom_module
        [1] => block_view
        [2] => custom_block_0
    )
2. _block_render_blocks
3. block_list
4. fusion_core_block_list
5. fusion_core_grid_info
6. fusion_core_preprocess_page
7. theme (page)
8. drupal_render
9. drupal_render_page
10. drupal_deliver_html_page
11. drupal_deliver_page
12. menu_execute_active_handler
aquariumtap’s picture

@girishmuraly - thanks for the backtrace info. Just to confirm, you're seeing the same block twice in your browser? Or is the concern that it's being processed twice?

girishmuraly’s picture

@aquariumtap, I should've been more clear. The block itself appears only once, so to the end user its all good. However, yes the concern is that the block is being processed twice.

esmerel’s picture

Status: Postponed (maintainer needs more info) » Active
adf1969’s picture

A few comments regarding this bug (it still exists for me, fusion_core 7.x-2.0-beta1.52-dev, and from looking at the 7.x-2.0 HEAD, it seems the code in fusion_core:template.php is the same as I have, so this problem has not been fixed).
Aquariumtap: While it is *true* that block_list() is statically cached, it is *also* true that at the end of building all the blocks, that "cache" is reset:
in: block.module: block_page_build()

    // Once we've finished attaching all blocks to the page, clear the static
    // cache to allow modules to alter the block list differently in different
    // contexts. For example, any code that triggers hook_page_build() more
    // than once in the same page request may need to alter the block list
    // differently each time, so that only certain parts of the page are
    // actually built. We do not clear the cache any earlier than this, though,
    // because it is used each time block_get_blocks_by_region() gets called
    // above.
    drupal_static_reset('block_list');

This is what causes the blocks to re-build/re-rendered all over again (every time, on every page-load).
This of course is a major performance issue, not to mention can create all sorts of other "issues" since it runs the code for each block TWICE on each page-load (the GMap issue above is just one example, and any CDN that you use in a block will also get used twice).

I'm looking at some options to fix this bug. If I figure out a way, I'll post my results here.

chirhotec’s picture

This is also breaking a multistep registration form I've created. The form works fine as a page within a fusion theme, or as a block with a different theme. Unfortunately, I need it to be a block, and I've already come along way in my custom fusion based theme.

webkenny’s picture

Status: Active » Postponed (maintainer needs more info)

Can someone point me to where we believe this is happening in Fusion? We can have a look.

webkenny’s picture

Status: Postponed (maintainer needs more info) » Active

Kris Bulman reports in #1905600: call to fusion_block_list in grid_info causes block alters to load twice which I've marked duplicate:

to reproduce, use the function hook_block_view_alter and add a dpm() call to any module (with DEVEL enabled):

function MODULENAME_block_view_alter(&$data, $block) {
  dpm($block);
}

You will see that krumo loads the block info twice. By removing the calls to fusion_core_block_list() in fusion_core_grid_info(), it loads them only once. It seems to be because block_list calls out to _block_render_blocks (which is needed here to determine if there is content in the block and if it should be listed). This behavior problematic because loading anything such as drupal_add_css or drupal_add_js in a block_view_alter, causes it to load twice on the page.

Of course, you can band-aid the problem by writing a bit of php that assures the function only loads once, however it shouldn't be necessary and will affect many module developers that use fusion.

We agree this is an issue. Any suggestions on how we should address it knowing that we need to know about the content/block list? Open to ideas.

KrisBulman’s picture

Does Fusion need to receive an array of blocks visible in a region, or just know the exact number of blocks in the region, or does it really only need to know if the region is empty or not?

webkenny’s picture

My feeling is, being a newer maintainer, that we only need to know if the region is empty or not so we can render it. I can't see any reason why we have to know what those blocks are. I do see that Fusion Accelerator may need some knowledge of it (given its skinning mechanism) but that's separate and the theme shouldn't depend on the module, IMO. We'll have a look into fusion_core and see what we can find for dependencies. Let us know if you find anything first.

aquariumtap’s picture

Hi webkenny - you're right, Fusion doesn't need to know what the block is, just if the region needs to be printed or not. Skinning is entirely the responsibility of Skinr or Accelerator, and the only special accommodation Fusion makes is to print the classes in the right place.

I agree with you that Fusion shouldn't depend on Accelerator, and I think that separation is there or *almost* there. The intention was for accelerator to depend on Fusion, but not the other way around. Without accelerator, the responsive features of Fusion would not be accessible, but the (non-responsive) theme should work.

webkenny’s picture

Assigned: Unassigned » webkenny

Fantastic then. We know what needs to be done. I will try to work a patch this week. Thanks for the clarification, aquariamtap. Good to get your feedback on these issues as we work through them.

KrisBulman’s picture

Then really, if the sidebars are empty the only value that needs to be passed is 1 or 0. You should be able to reliably determine if sidebars are rendered from preprocess_page .. doing a check there, then passing the resulting value to the grid function (with a helper function) should do what you're looking for.

Gabriel’s picture

Is there a work around for this? Or a way I can tell it's the first call to the block and not the second? I'm rendering out advertising in views on the page via a hook_block_view() and they are being rendered twice as I'm using drupal_add_js() to insert the ad call. I notice any returned values during the first call are thrown away, but the drupal_add_js call can't be disposed of in the same way.

Any suggestions would be greatly appreciated.

gcassie’s picture

BetoAveiga’s picture

@Gabriel I "fixed" this but I'm not using context. Maybe this link could help you to find a solution https://drupal.org/node/2178485

dreamingX’s picture

Issue summary: View changes

This worked for me (thanx to BetoAveiga):
Changing the lines in template.php:

  if (module_exists('block')) {
    $drupal_list = block_list($region);  
  }

to:

  if (module_exists('block')) {
    $drupal_list=array();
    $list_of_blocks_per_region = _block_load_blocks();
    if (isset($list_of_blocks_per_region[$region])) {
      $drupal_list = $list_of_blocks_per_region[$region];
    }
  }
Poieo’s picture

Assigned: webkenny » Unassigned
Priority: Major » Normal
Status: Active » Needs review
FileSize
961 bytes

Attached is a patch based on #25. Please test this to see if it resolves the issue without creating more.

  • Poieo committed 3ed5b37 on 7.x-2.x
    Issue #1172280 by Poieo: Fixed blocks rendering twice.
    
Poieo’s picture

Status: Needs review » Closed (fixed)
Poieo’s picture

Status: Closed (fixed) » Needs work

This has been rolled back due to another issue it caused: #2340749: Main content doesn't adjust properly with empty sidebars

Still looking for a fix that doesn't break other functionality.

  • Poieo committed 3ed5b37 on 7.x-3.x
    Issue #1172280 by Poieo: Fixed blocks rendering twice.
    
  • Poieo committed 36bc458 on 7.x-3.x
    Issue #2340749 Poieo: Main content was not adjusting for empty sidebars...

  • Poieo committed 3ed5b37 on 8.x-1.x
    Issue #1172280 by Poieo: Fixed blocks rendering twice.
    
  • Poieo committed 36bc458 on 8.x-1.x
    Issue #2340749 Poieo: Main content was not adjusting for empty sidebars...