Summary as of November 2014

This problem has been fixed in D7 and D8.
See #43 for D7 repro steps, and #60, #61 and #62 confirming fixed in D7.

It is still a bug in D6.34.
The repro steps are in #40. Patch re-rolled in #68 with same code changes as in #40 but comment wrapping corrected.


Original starting post from February 2007, Drupal 5.1

I have upgraded from 5.0 to 5.1 and previously had several user-defined blocks with content and visibility set using php. They still function fine in 5.1 but when using admin -> block -> configure the actual php code is no longer shown in the text area. Likewise the title entry field is blank, even though the correct title is displayed in the real block. ie the data is stored OK in the db, but it is just not being shown in the configure entry form. I have proved I am looking at the same block by entering a new title, which then gets shown on the real block.

Ask if you want more info,
Jonathan

CommentFileSizeAuthor
#68 _115596_68_ab.consistent_block_settings-D6.patch1.98 KBjonathan1055
#68 _115596_68.simpletest.block_consistency-D6-do-not-test.patch4.29 KBjonathan1055
#68 _115596_68.consistent_block_settings-D6.patch1.97 KBjonathan1055
#65 _115596_65.simpletest.block_consistency-D6.patch4.29 KBjonathan1055
#65 _115596_65.consistent_block_settings-D6.patch1.97 KBjonathan1055
#60 1 - after minimal install.jpg471.05 KBjonathan1055
#60 3 - after enbling stark.jpg700.79 KBjonathan1055
#60 4 - after enabling blog module.jpg739.93 KBjonathan1055
#60 5a - admin structure blocks.jpg743.47 KBjonathan1055
#60 5b. customising block.jpg751.13 KBjonathan1055
#43 D7 step 1 after install.jpg123.84 KBjonathan1055
#43 D7 step 2 after listing blocks.jpg173.6 KBjonathan1055
#43 D7 step 3 after enabling Stark.jpg278.16 KBjonathan1055
#43 D7 step 5a after listing blocks in Garland.jpg279.65 KBjonathan1055
#43 D7 step 5b after configuring Garland block.jpg31.28 KBjonathan1055
#43 block showing in Garland.jpg57.53 KBjonathan1055
#43 block showing wrongly in Stark.jpg163.47 KBjonathan1055
#43 _D7_block_admin.copy_existing_settings.patch.txt1.06 KBjonathan1055
#53 115596.block_admin.copy_existing_settings-D7.patch1.26 KBohnobinki
#46 D7 results of 1-5 after patch applied.jpg94.57 KBjonathan1055
#44 115596.block_admin.copy_existing_settings-D7.patch1.06 KBjonathan1055
#40 step1 immediately after install.jpg47.98 KBjonathan1055
#40 step2 after adding custom block.jpg89.73 KBjonathan1055
#40 step3 after enabling bluemarine.jpg161.95 KBjonathan1055
#40 step5 after configuring Recents Posts block.jpg180.89 KBjonathan1055
#40 step6 after deleting Block1.jpg209.88 KBjonathan1055
#40 step7 after listing Bluemarine blocks.jpg172.58 KBjonathan1055
#40 step8 bluemarine showing block on my account.jpg63.64 KBjonathan1055
#40 step9 garland showing customised blog block.jpg93.4 KBjonathan1055
#40 _block_settings.order_by_bid.patch.txt728 bytesjonathan1055
#40 _block_settings.existing_block_values.patch.txt1.7 KBjonathan1055
#40 step7 after listing Bluemarine WITH PATCH.jpg180.49 KBjonathan1055
#35 _block_visibility_theme.D613.patch.txt3.59 KBjonathan1055
#26 block_theme_patch.txt780 bytesfmccormick
#25 block_theme_patch.txt927 bytesfmccormick
#9 block_visibility_theme.patch2.73 KBjoshk
#6 block_visibility_theme.patch3.73 KBjoshk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055’s picture

Project: Drupal core » Administration
Version: 5.x-dev » master
Component: block.module » Code
Assigned: joshk » Unassigned
Status: Needs work » Active

Sorry, the titles and meaning of the drop-down menus when submitting a bug are not very clear. This bug is nothing to do with the administration module (I have not even downloaded that one). I selected 'administration' thinking that 'project' meant the area of Drupal to which the bug relates. It can be moved to another project if more appropriate.

Jonathan
(previously known as mr_splosh - to aid references to this name in the thread)

dreed47’s picture

Project: Administration » Drupal core
Version: master » 5.1
Component: Code » block.module

moving to Drupal project

Anonymous’s picture

I noticed this same bug.

I took a look in the module code and this is what seems to be happening:
- As Mr_Splosh said, the data looks fine in the DB
- However, the settings for each block seem to be stored on a per theme basis (IE you can have the same block title for different themes)
- I noticed the code which pulls the data from the DB for the block config form (admin -> block -> configure) does not specify which theme to use in the data lookup
- This is the problem I think (at least it was in my case) because it was actually pulling the data from another theme's block (which was blank) rather than my current default theme

In my own code I just edited this file to pull from the block table using my current theme as a parameter (along with the module type and delta) and this seemed to fix this bug

joshk’s picture

Version: 5.1 » 5.3
Assigned: Unassigned » joshk

Bumping and taking this on.

joshk’s picture

Ok, I have a working fix: basically we need to pass another arg in the url. I will post a patch shortly.

joshk’s picture

Version: 5.3 » 5.x-dev
Status: Active » Needs review
FileSize
3.73 KB

Here's a patch that will fix this based on 5-dev. Shoud work for 5.3 installs too...

Gábor Hojtsy’s picture

How come you can get rid of isset($block['throttle']) ? $block['throttle'] : 0 in the code, just replacing it with $block['throttle']?

drumm’s picture

Status: Needs review » Needs work

Yes, changing that likely adds a slew of PHP notices when throttle module is off.

joshk’s picture

Status: Needs work » Needs review
FileSize
2.73 KB

Good catch. I did a full line-replacement from my existing install, so likely missed that update. I've gone back and re-rolled the patch from a fresh CVS checkout.

joshk’s picture

Also, fwiw this is already fixed for drupal 6, so no need to worry about it going fwd.

Pasqualle’s picture

Status: Needs review » Reviewed & tested by the community

This works, thanks..

drumm’s picture

Status: Reviewed & tested by the community » Needs review

I having trouble reproducing the original issue. In Drupal 5, I am not sure if blocks should get custom settings per-theme. The navigation does suggest they might. However, any module-added settings are going to be set for every theme. Having every setting apply to every theme would be consistent.

Pasqualle’s picture

I spent more than an hour to reproduce this bug, it is not easy.

just copy paste the links to your address bar, do not need to navigate through the menu

1. clean drupal 5.5 install
2. admin/build/themes - click "Save configuration"
3. admin/build/block/list/garland - do nothing
4. admin/build/block/configure/user/3 - fill textfield "Pages:" with "anything" and click "Save block"
5. admin/build/block/list/bluemarine - do nothing
6. admin/build/block/add - fill textfield "Block description:" with "anything" and click "Save block"
7. admin/build/block/configure/user/3 - BUG The textfield "Pages:" is empty

So the problem is in point 5 that we got inconsistent rows in table {blocks} for
select pages from {blocks} where module = "user" and delta = 3;
the pages column contains "anything" for garland and empty for bluemarine, because blocks for bluemarine were initialized at this point with default values.

The trick in point 6 that it somehow changes the order of rows in DB, so user/3 moves to the end of the table for garland theme.

and this code (which is used to display the block visibility) returns the first row found, which is now the empty bluemarine

$edit = db_fetch_array(db_query("SELECT pages, visibility, custom, title FROM {blocks} WHERE module = '%s' AND delta = '%s'", $module, $delta));

The reproduction of the problem is not obvious, but it happens many times, if you add new blocks to your site.
I use this patch for a month now on live site without a problem.

And yes, you can have custom settings per-theme with this patch, but I like it. The design of {block} table suggest this behavior.

jefftrnr’s picture

I'm using Drupal 5.7 with several themes running via the project/domain module. This bug was a HUGE problem. So far, it seems solved with your block_visibility_theme.patch. This fix was crucial! Thanks so much... please be sure to have this patch included in the next Drupal 5 update!

drumm’s picture

Status: Needs review » Needs work

Custom settings per-theme is not in Drupal 6. This bugfix must implement what is in Drupal 6, since the bug is fixed there. If it is done differently, this is a new feature and will cause configuration changes when you upgrade to Drupal 6.

Pasqualle’s picture

Custom settings per-theme is not in Drupal 6

I don't understand you. In Drupal 6 you have custom settings for blocks per theme. Every theme has its own block visibility setting..
I did not checked the code, if it is same as this patch, but I know that the functionality is the same (or similar).

drumm’s picture

If I click configure from http://drumm-laptop/6/admin/build/block, I go to:
http://drumm-laptop/6/admin/build/block/configure/user/1

If I click configure from http://drumm-laptop/6/admin/build/block/list/bluemarine, I go to:
http://drumm-laptop/6/admin/build/block/configure/user/1

These are the same, so how would Drupal 6 accomplish custom block settings per-theme?

Pasqualle’s picture

sh*t, you are right..
but this is so wrong. or is not? why we force same settings for every theme?

but, yes, the patch needs work, should be the same functionality as it is in drupal 6

jefftrnr’s picture

Does anyone know how this block-settings-per-theme issue is accomplished in Drupal 6? I'd like to find out if this can be resolved in Drupal 5, either with this patch or some other. I'm now running drupal 5.10 and I don't think it's fixed yet.

Why doesn't this current patch work? Does it change the table schema that much? It seems to only make sure the correct columns are in the WHERE clauses. I'm not sure I follow drumm's concerns with this patch causing problems for the upgrade path to Drupal6. But, if I'm using the patch currently, will I have trouble upgrading the block db table when I want to use Drupal 6 later on?

Pasqualle’s picture

this patch does not change the table schema. If you use this patch you may have to manually reset the visibility settings in all blocks, after you upgrade to Drupal 6 (but if you encounter this bug, then I think you have to do the D6 reset even if you do not use this patch, because the current D5 block visibility code is simply broken)..

the only "problem" with this patch is that it introduce a new "feature": Custom block visibility settings per-theme.. Such feature was never approved for Drupal core..

greenmachine’s picture

This problem has really only cropped up for me on a larger, more complex site (but not other sites of similar complexity). I have no idea why. My approach to fixing was a little different, didn't involve changing core code. Instead I built something that will warn when the rows representing one block (across multiple themes) have become desynched (title/visibility/pages no longer is the same for each theme). Here's the code:

Into hook_menu goes:

    $items[] = array(
      'title' => 'Block Fixer',
      'path' => 'blockfix',
      'callback' => 'drupal_get_form',
      'callback arguments' => array('blockfix'),
      'type' => MENU_CALLBACK,
      'access' => user_access('access administration pages'),
     );

Into hook_form_alter goes:

  if ($form_id == 'block_admin_configure') {
    $current = array(
      'module' => $form['module']['#value'],
      'delta' => $form['delta']['#value'],
      'pages' => $form['page_vis_settings']['pages']['#default_value'],
      'visibility' => $form['page_vis_settings']['visibility']['#default_value'],
      'title' => $form['block_settings']['title']['#default_value'],
      );
    $keys = array('pages', 'visibility', 'title');

    $r = db_query("SELECT * FROM blocks WHERE module = '%s' AND delta = %d", $current['module'], $current['delta']);

    $mismatches = array();
    while ($block_instance = db_fetch_object($r)) {
      foreach($keys as $key) {
        if ($current[$key] !== $block_instance->$key) {
          $mismatches[$block_instance->theme][] = $key;
        }
      }
    } // end while

    if (count($mismatches) > 0) {
      $form['mismatch_warning'] = array(
        '#weight' => -10,
        '#value' => '<div class="messages error">WARNING! This block has mismatched data in the database. Please use the ' . l('block mismatch fixer', 'blockfix/' . $current['module'] . '/' . $current['delta']) . ' to reconcile these differences before editing this block</div>',
        );
    }
  }

Finally a new function for the blockfix form and a corresponding submit function:

function blockfix($module, $delta) {
  $r = db_query("SELECT * FROM blocks WHERE module = '%s' AND delta = %d", $module, $delta);
  $existing = '';
  while ($block = db_fetch_object($r)) {
    $themes[$block->theme] = $block->theme;
    $existing .= '<strong>Theme: ' . $block->theme . '<br /></strong>';
    $existing .= 'Title: ';
    $existing .= ($block->title) ? $block->title : '<em>none</em>';
    $existing .= ' Visibility: ' . $block->visibility . '<br />';
    $existing .= 'Visible pages or PHP code: <br />';
    $existing .= ($block->pages) ? '<textarea cols="50" rows="4">' . $block->pages . '</textarea>' : 'NONE';
    $existing .= '<br /><br />';
  }
  $form['existing'] = array(
    '#value' => $existing,
   );

  $form['delta'] = array(
    '#type' => 'value',
    '#value' => $delta,
    );
  $form['module'] = array(
    '#type' => 'value',
    '#value' => $module,
    );

  $form['theme'] = array(
    '#type' => 'select',
    '#title' => 'Select theme to make authoratative',
    '#options' => $themes,
    '#required' => TRUE,
    '#description' => 'Select the theme that corresponds to the entry above that represents the block configuration that is correct. This will overwrite the inaccurate values associated with other themes',
    );
  $form['submit'] = array(
    '#type' => 'submit',
    '#value' => 'Submit',
    );
  
  return $form;
}


function blockfix_submit($form_id, $form_values) {
  $r = db_query("SELECT * FROM blocks WHERE module = '%s' AND delta = %d AND theme = '%s'", $form_values['module'], $form_values['delta'], $form_values['theme']);
  $block = db_fetch_object($r);
  
  db_query("UPDATE blocks SET visibility = %d, pages = '%s', title = '%s' WHERE module = '%s' AND delta = %d", $block->visibility, $block->pages, $block->title, $form_values['module'], $form_values['delta']);
  drupal_set_message('Corrected mismatched/inconsistent values for this block');
  
  return 'admin/build/block/configure/' . $form_values['module'] . '/' . $form_values['delta'];
}
jonathan1055’s picture

Wow! It is nearly two years since I raised this bug, and it is still an ongoing problem.
Or rather, there is a similar problem but it is reversed. I now have a new site in D6.8 and when adding an existing block to a new theme, the block visibility is lost and the title is blank when browsing the site. Then, only after a 'configure' and then 'save' with no changes, does the title get shown and the visibility code enacted on. Is this the expected behaviour? ie every block must be edited and saved before the new theme operates correctly?

Pasqualle’s picture

@Mr_Splosh: is it an upgraded D5 site, or you can reproduce the bug with a clean D6 install? If you can reproduce it with a clean D6 install, then please provide the reproduction steps.

robschmitt’s picture

Gotta say, it's pretty ridiculous that this issue has never been addressed in core. I've just run into this problem again -- and once again on a large site with dozens of blocks where I can ill afford my blocks randomly moving to different positions.

fmccormick’s picture

FileSize
927 bytes

Hit this again on our latest project ... after hunting through the code it came down to the theme having been set before the block module is initialised (block module is setting a custom theme and if that doesn't get initialised properly
_regions isn't set which does bad things to your block table)

The culprit in our case was a module calling path_to_theme() in its menu - this calls init_theme() before block.module gets a chance to set the custom theme.
This would also rear its head if you do anything that requires a theme in the main module code (i.e. if it fires in the require_once)

We removed the offending path_to_theme() call which fixed our problem, however I've also implemented a security check which stops anything calling init_theme in the drupal_bootstrap segment.

The following code was added to init_theme() (in theme.inc) just after the globals:

  $backtrace=debug_backtrace();
  if($backtrace[sizeof($backtrace)-1]['function']=='drupal_bootstrap'){  	
  	die("Error: Attempt to initialise theme in bootstrap phase. This is usually caused by a call to path_to_theme() within a hook_menu or a module file.\n<xmp>".
  		print_r($backtrace,true)."</xmp>");
  }

Patch file attached for 5.15

fmccormick’s picture

Project: Administration » Drupal core
Version: master » 5.x-dev
Component: Code » block.module
Assigned: Unassigned » joshk
Status: Active » Needs work
FileSize
780 bytes

Looks like the above patch does bad things to redirects. I've modified it to only trigger if you're in admin/build/block or a subdirectory. Probably a better way to do that...

  $backtrace=debug_backtrace();
  if(arg(0)=='admin' && arg(1)=='build' && arg(2)=='block' && $backtrace[sizeof($backtrace)-1]['function']=='drupal_bootstrap'){  	  	
  	die("Error: Attempt to initialise theme in bootstrap phase. This is usually caused by a call to path_to_theme() within a hook_menu or a module file.\n<xmp>".
  		print_r($backtrace,true)."</xmp>");
  }

Nico_0’s picture

I upgraded from 5.11 to 5.12 recently and now I am experiencing the same problems (title and visibility setting of block dissapearing).

What should I do?

Pasqualle’s picture

Nico_O try the patch in #9 and read comment #20..

Nico_0’s picture

Thanks,

I applied the patch, and the visibility settings are showing again.

Tresler’s picture

Status: Needs work » Closed (won't fix)

As far as I can tell, this isn't an issue in D6 and is beyond fixing in D5. Marking won't fix, and moving on. Re-open if there is more to add to this.

Pasqualle’s picture

Status: Closed (won't fix) » Needs work

what kind of reasoning is that? D5 is still supported, if someone creates an acceptable patch it still can be fixed..

jonathan1055’s picture

@30
Hi, yes this is still an issue in D6. As far as I know it as not been fixed.

@31
I agree, that whilst D5 is still supported we should try to keep it running correctly.

Tresler’s picture

Ok, apologies, thought this was a dead issue. My mistake.

landike’s picture

THANKS PEOPLE !!!!!!!!!!!!

very helpful page..

jonathan1055’s picture

Version: 5.x-dev » 6.x-dev
FileSize
3.59 KB

I've just run into this problem again and it is definitely not fixed as of D6.13 (unless Joshk from #10 can show me otherwise). I think we need to understand how D6 is intended to function before we can suggest fixes, and then port them back to D5. There is some urgency now, because D7 is coming up fast.

Does anyone know (or how can we find a difinitive answer) if custom theme settings per block is intended to be a D6 feature? The schema of the block module implies that it could be, otherwise there would be a separate table for some of the block settings and they would not be duplicated for every row in {blocks} just to add a different theme. However, the current functionality for block visibility by role uses the separate {block_roles} table which is linked by delta, which does allow separate settings per theme but often there is only one delta value across all themes, eg my search block has delta=0 for all themes, so whenever I change the visibility by role it changes for all themes. Other blocks have differing delta values which inplies that their visibility by role can differ across themes. So I can't tell what was actually intended, as it is a mixture of both.

Having custom block settings for different themes would certainly be a useful feature, for example, to have a 'standard' theme with most blocks displayed by default and a 'minimal' theme with most blocks not displayed but the user can turn them on if required. Or the search block in a 'standard' theme could have the title Search but in a 'Casual' theme it could be Go fetch this! It would certainly be quite easy to have this functionality, as it is almost there already! If this feature is intended then the code in the patch in #9 should be implemented. Also we could consider the excellent idea from Greenmachine in #21 which allows the admin to select from the existing differing settings - this could be very useful when adding a new block. The check from Fmccormick in #26 could also be added.

On the other hand, if custom settings per theme are not intended then we need to fix the code which initialises a new block when adding it to a theme so that the existing settings are copied to the new row in {blocks}. As it stands in D6, if you have configured a block in your default theme, then add it to a second theme the config settings are not copied forward, then when you save this new block, the settings are written over both the new and the old block rows, thus losing your previous configuration inputs.

I have applied the code changes in the patch in #9 to D6 (the functions have moved from block.module to block.admin.inc and a few $form keys needed to be changed, but the principle is identical and the base code has not changed at all from D5. I've attached the patch if anyone wants to try it out.

Jonathan

Pasqualle’s picture

so, to answer your question. feature requests for D6 are not allowed. and also too late for D7.
So, "Custom block visibility settings per-theme" can be asked for Drupal 8 only, and please open a new issue, as this one should be for fixing the bug (found in D5).

I did not see this bug in D6, and the repro steps in #13 works correctly.. can you provide steps how to reproduce the bug in D6?

jonathan1055’s picture

Hi,
I've got all the information prepared, with screen grabs and a patch, but strangely drupal.org is not allowing me to preview the post before submitting. The preview just returns my input textarea with my text, but no preview. It fails on other issue queues too. I'm not going to post without a preview, so it will have to wait! Also the file uploads get stalled at random places, sometimes after the the third file, sometimes after the second, so my post would be less useful without these. I posted Cant preview posts in issue queues to the general forum.

Pasqualle’s picture

landike’s picture

People.. JUST LET ME KNOW - this issue is fixed for Drupal 6.14 ??????

Will it be updated in Drupal 7?

jonathan1055’s picture

Hi Pasqualle,
Yes I know that new features are not allowed, I was just trying to work out whether this was already intended as a feature which was not quite working correctly, given the comments in #13-19. OK we can take it that custom settings are not a feature and therefore we must fix D6 and D5 accordingly. There are two problems that have been discussed in this thread, and the following steps will illustrate both. Creating a custom block, then deleting it later is just a controlled way to simulate what can happen when modules which provide blocks are enabled, then disabled and uninstalled.

The reproduction steps are:

  1. Start with a clean install of Drupal6 (I used 6.14)
  2. Admin - Blocks - add new block, with Block1 for both the description and title, and save it. There is no need to assign it to a region
  3. Admin - Themes - enable Bluemarine and save
  4. Admin - Modules - enable Blog module and save configuration
  5. Admin - Blocks - configure 'Recent Blog Posts' and set title = 'Custom title for blog', set visibility to only show on <front> page. Do not assign it to a region yet.
  6. Admin - Blocks - delete Block1
  7. Admin - Blocks - select Bluemarine tab to list blocks in that theme
    (the problem has now been created - the next two steps are just to illustrate it)
  8. Admin - Blocks - enable Recent Posts for Bluemarine into the right sidebar and save (do not configure yet)
  9. Admin - Blocks - click Garland tab and enable Recent Posts into the right sidebar (do not configure yet)

Explanation of the above steps with screen grabs of the {blocks} table:

  1. The clean install starts with Garland as the only theme enabled and the blocks table has 3 rows (step1 image)
  2. After the intermediate step of listing the blocks we will have had 9 rows in the blocks table (3 from above + 6 added by Admin - Blocks). Then after adding a custom block we have 10 rows (step2 image)
  3. After enabling Bluemarine we have 20 rows in the table, ie the above 10 repeated for the new theme (step3 image)
  4. After enabling a module which provides a block, the block table is not immediately changed
  5. After configuring the Recent Blog Posts in the default Garland theme, we get one row added to the blocks table, making 21 rows (step5 image)
  6. After deleting our new Block1 we have 19 rows in the blocks table because bid 10 and 11 are removed (step6 image)
  7. After selecting the Bluemarine tab, any existing blocks for Garland which do not have a row for Bluemarine are initialised. We now get a new block with bid=22 but it has been added between row 9 and row 12 ie. into the gap left by the deleted block. It only has default values, ie no title and visibility settings (step7 image)

We now have the two problems encountered at various stages in this thread. First, the block in Bluemarine does not contain the custom settings that were applied to the block in Garland. Unless and until the admin configures the block in Bluemarine, it will be shown on every bluemarine page and will not have the custom title. See step8 image showing the un-customised block on the 'my account' page versus the customised block in Garland (step9 image).

Secondly, if the admin attempts to configure the block, the data from the Bluemarine row is the one that gets extracted into the configuration form, not the row from the Garland theme that was customised in step 5. Thus there is no way to recover the previous customised settings. As soon as the block configuration form is saved then the new data will be written over both the rows in the blocks table, thus erasing the original data completely.

A simple solution for the second problem in to order the sql select by block id. Only the first row is returned, and currently there is no ORDER BY, so the first matching row in the table is used (which in this example is the empty row inserted between bid 9 and 12). With the select ordered by block id then the older row with lower block id (and with the customisations) would be returned. This allows the admin to recover the original customisations, which when saved again will be propagated to all rows for that block, across the other themes. The attached patch _block_settings.order_by_bid does this, which makes the change in block_admin_configure() within block.admin.inc

For the first problem, to give a new block the existing customised settings and avoid it only getting the defaults, I suggest a change to _block_rehash() in block.module. This is where new rows are added to {blocks} when they are missing for a specific theme. The attached patch _block_settings.existing_block_values retrieves an existing row for this block and uses the data for the new row.

I have not checked whether these steps and the fixes are the same for D5, but the code base is very similar. I will leave that until after my suggestions have been reviewed and we agreed about the correct method for fixing D6.

Jonathan

Pasqualle’s picture

From the repro steps it seems it is a bug in D6 also.

My first question would be: is it a bug in D7? because everything should be fixed in the latest version first and backported. The solution would be properly reviewed. There were some block changes in D7, so I do not know..

The "order by bid" solution seems like a quick fix which could still break sometime, but I am not sure if I could create a valid case when it would break. Like if I would completely uninstall a theme, with removing the records in the blocks table manually (or with a special contrib module), then I would lose the block settings which are not yet propagated to other themes.

my personal opinion: if it is a bug in D7 then maybe it would be good, to still ask for the feature request in D7 as it could be a much cleaner solution..

jonathan1055’s picture

Yes I get your point about checking D7 first. I will try to look into that.

The simple fix with 'order by' is only to help those who currently have mis-matched rows. The main fix will stop this error happening in future, so they are both required for D6.

jonathan1055’s picture

Yes, I've tested this on D7 and the problem exists there too. Some work has been done in block.admin which does improve things though. Here are the repro steps:

  1. Start with a clean install of Drupal 7 minimal (I used dev dated 20th Sept). The {block} table has 5 rows.
  2. Admin - Structure - Blocks. The block table now has 12 rows. No need to add new custom block1 as for D6.
  3. Admin - Appearance - enable Stark. The block table now has 24 rows (12 for Garland, 12 for Stark)
  4. Admin - Configuration and modules - Modules tab - Enable Blog module (no change to block table)
  5. (a) Admin - Structure - Blocks. This produces 1 new row, bid=25, for the Blog block in Garland only
    (b) Configure 'Recent Blog Posts', set title = 'Custom title for blog', set visibility to only show on <front> page, set user specific 'show but allow to hide'

After the custom configuration is saved and bid=25 is updated, a new row is added for Stark, with bid=26. This has the 'pages' value copied, but 'custom', 'visibility' and 'title' not copied, which gives us a mis-match across themes. See image step 5b. If the blocks are now assigned to regions [and a blog content post is written ] then in Garland the block is displayed on the front page with the customised title, but in Stark it is shown on every page except the front page and it only has the default title. The last two images show this.

A fix is required to the code in block_admin_configure_submit() in block.admin.inc, which creates new rows for the other enabled themes.
I have made a patch for this, and changed the version to 7.x-dev.

Jonathan
[edit on 23.09.2010 only for fixing a spelling mistake]

jonathan1055’s picture

Version: 6.x-dev » 7.x-dev
FileSize
1.06 KB

Thought I'd try uploading the same patch for automated testing. Not tried this before.

Pasqualle’s picture

I think we will need a simpletest for this one, but it looks like it will be quite complicated. Any simpletest guru here?

jonathan1055’s picture

The D7 patch was subjected to 427 Block tests and passed them all. Attached is the results after applying the patch and re-running repro steps 1-5.

Jonathan

Pasqualle’s picture

Ok, but those 427 tests pass even without the patch.. We need at least one test which would fail without the patch, so we won't reintroduce this bug in the future..

cburschka’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ modules/block/block.admin.inc	2009-09-26 12:55:27.000000000 +0100
@@ -382,14 +382,17 @@ function block_admin_configure_submit($f
+    // Update the customised settings for all enabled themes. This will create new rows if required.

Comments should wrap after 80 characters.

Also, needs test case.

I'm on crack. Are you, too?

sun’s picture

Anyone up to re-roll this patch?

exaboy’s picture

I need this patch for D6.17...

jonathan1055’s picture

Hi,
The patch I gave in #35 still applies OK in 6.16, and I've just checked and block.admin.inc is identical from 6.16 to 6.17 so you can use it.

Jonathan

ohnobinki’s picture

+1

ohnobinki’s picture

Status: Needs work » Needs review
FileSize
1.26 KB

The requested re-roll against HEAD.

mrfelton’s picture

Status: Needs review » Needs work

With the patch at #35 applied to D6, block multilingual settings do not save properly.

mrfelton’s picture

Status: Needs work » Needs review

Ignore my previous comment - it's unrelated, and due to the i18n issue in #766678: The "All languages" option can't save back from the block Multilingual settings.

jonathan1055’s picture

Issue tags: +Needs tests

#53: 115596.block_admin.copy_existing_settings-D7.patch queued for re-testing.

Just checking the latest status. Would be good to get this accepted, then we can return to work on the D6 patch.

mcarbone’s picture

Version: 7.x-dev » 8.x-dev
Assigned: joshk » Unassigned
Issue tags: +Needs issue summary update
xjm’s picture

Status: Needs review » Needs work

This no longer applies.

gpk’s picture

Title: Block title and visibility php gets blanked out in block->configure » Block title and visibility settings can get blanked out in block->configure

Is this a problem in 8.x still? Have certainly seen it in earlier versions tho'...

(also reworded title slightly)

jonathan1055’s picture

I think it might be fixed in D8 and D7 actually. I have just tried my repro steps from #43 in D7.12 and everything appeared to work ok. I have got screen grabs of the block table at each point, see below. However, I want to go back and compare the source code at various releases to see exactly when it was fixed, as there is nothing in this thread which says anything about a commit to D7. It must have been done in a different issue.

If it is fixed then we can port the correction back to D6 to fix the bugs there.

xjm’s picture

Issue tags: +Needs manual testing

Hmm, other than the fix to transaction handling in #1022416: Wrong use of db_transaction() in block module, these lines haven't changed since #337212: DBTNG: Block module, so if it was fixed, it was not fixed in this way.

So, let's get one other person confirming this isn't reproducible in D7 and, if so, bump it back to D6?

cossovich’s picture

Status: Needs work » Needs review

I can't reproduce this bug on D7 (using a fresh copy of 7.12) following the steps at #43.

The difference is that at step 5a I get 2 new rows for the block table, bid=25 for bartik and bid=26 for stark. After the custom configuration step at 5b (customising title and visibility settings for bartik theme) the custom options are also saved for stark in the bid=26 row.

Seems as though this is fixed for D7.

xjm’s picture

Version: 8.x-dev » 6.x-dev
Issue tags: -Needs tests

Thanks @cossovich! Based on #60 and #62, moving this issue back to D6.

jonathan1055’s picture

My two patches in #40 are (probably) still valid as an easy fix and simple to understand. May need a re-roll though.

jonathan1055’s picture

Title: Block title and visibility settings can get blanked out in block->configure » Block custom title, visibility and pages settings get blanked out in block->configure
Issue summary: View changes
Issue tags: -Needs issue summary update
FileSize
1.97 KB
4.29 KB

Here's a re-rolled patch - same code changes as in #40 but comments now wrap at 80. In some set-ups the problem is slightly mitigated by newer database versions which do not add new rows in the space vacated by deleted rows. So if you follow the repro steps in #40 you may find the second problem is eased, where you can recover from the lost settings. But the main problem is still there - that blocks lose their custom title, visibility and pages values when a new theme is enabled.

I've also written an additional simpletest case in the /simpletest/tests/block.test file, if anyone would like to see the problem demonstrated automatically.

The last submitted patch, 65: _115596_65.consistent_block_settings-D6.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 65: _115596_65.simpletest.block_consistency-D6.patch, failed testing.

jonathan1055’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.97 KB
4.29 KB
1.98 KB

The last submitted patch, 68: _115596_68.consistent_block_settings-D6.patch, failed testing.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.