Not sure if this is the way it's supposed to work, but does seem buggy to me.

When you add a new Menu Block, the CSS class(es) text field is not there. Only when you save the block and go back to edit it does it show up. This does not happen when you add a regular block.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vchen created an issue. See original summary.

jennypanighetti’s picture

I'm not sure if this is normal or not either, but this is the way it is in version 2.1 as well.

Benner’s picture

I see that it shows up if I create a normal block but when I create a menu block it is missing. Does not show up at all in the source.

DYdave’s picture

Priority: Minor » Normal

ok, thanks a lot for reporting this issue guys.

I'll take a quick look over the week end and see what we could do.

Cheers!

DYdave’s picture

Title: Missing text field for Adding Menu Block » Menu Block integration: Missing text field for Adding Menu Block
Status: Active » Needs review
Issue tags: +simpletests
FileSize
6.48 KB

Hi guys,

Quick follow up on this issue.

Assessment of the issue:

So I managed reproducing the issue very easily with:

  • menu_block-7.x-2.7+1-dev
  • block_class-7.x-2.2+1-dev

Install Block Class and Menu Block, then add a new Menu Block at:
admin/structure/block/add-menu-block
Result: The CSS Field is nowhere to be found: it is not displayed on the page.

However, when the block is edited, for example at:
admin/structure/block/manage/menu_block/1/configure
The field is visible and it is possible to set the CSS classes which are then saved and displayed properly in block's markup.

Basically, the problem comes from the if statement in block_class_form_alter, line 70:

<?php
  if (user_access('administer block classes') && ($form_id == 'block_admin_configure' || $form_id == 'block_add_block_form')) {
?>

and respectively the if statement in block_class_form_submit, line 92:

<?php
  if ($form_state['values']['form_id'] == 'block_admin_configure' || $form_state['values']['form_id'] == 'block_add_block_form') {
?>

because the menu block creation form page at admin/structure/block/add-menu-block uses a different form ID menu_block_add_block_form.
 
So the starting if test in block_class_form_alter detailed above fails and nothing happens.
But once the new menu block has been created, the editing process goes through the common/standard block configuration form page whose ID is block_admin_configure, so everything works fine.
 

Proposed solution:

The simplest and most direct way of fixing this issue would be to add Menu block creation form ID to these if statements.
For now, this is the solution that we're probably going to prefer.
 
However, I'm guessing this issue would most likely happen with many other block-related modules, such as Nice Menus, Superfish, and any other module that would add its own blocks through the block administration interface.
Since it doesn't seem like any similar issues have been reported for other block related modules, I suppose we could probably settle for a simple/quick solution for now, as suggested above.
 
Otherwise, we could potentially use a wildcard/regular expression in the if statements above, to check if the form ID would end with _add_block_form, for example.

SimpleTests:

Since I assume Menu block is a very popular module, I thought we'd better go the extra mile and have this compatibility issue automatically tested in code.
 
So I created a new test case for Block Class' integration with Menu Blocks: BlockClassMenuBlockTestCase.
The tests are very straight forward:

  • Start with the creation of a new Menu Block, with CSS classes, through the block admin interface.
  • Check if the Block was successfully created.
  • Check if the Block CSS classes could be found in the markup.
  • Run the standard tests on the existing Menu Block created above.
    Edit the block, change the class and check with the anonymous user.

Note that we could use Menu Block's API and Menu Block Export to import/declare a menu block from code. But, the whole point of this issue is related with the initial creation process, in particular going through menu block's creation form menu_block_add_block_form. So we'd better be testing the creation process that would be the most likely to be followed by any users.

Please find attached to this comment a patch against block_class-7.x-2.x at 7792ecd, which implements the changes detailed above to fix this issue.
File attached as: block_class-menu-block-integration-2639340-5.patch.

Feel free to let me know if you would have any questions, objections, concerns, or issues, on the patch, suggested approach or the module in general, I would surely be glad to provide more information or explain in more details.

Thanks again very much for your interest, testing/reporting this issue and in advance to all for your testing, comments and reviews.
Cheers!

  • DYdave committed 43978a6 on 7.x-2.x
    Issue #2639340 by DYdave: Menu Block integration: Fixed missing CSS text...
DYdave’s picture

Status: Needs review » Fixed

Alright, since the tests passed, I went ahead and got the patch from #5 committed against block_class-7.x-2.x at 43978a6.

Until the next stable release, updating your module to the latest 7.x-2.x-dev version should fix the problem on your sites.

If at some point, adding extra form IDs to block_class_form_alter isn't sufficient, for example if another module encounters the same issue, it shouldn't be a problem to check for a pattern instead of a fixed key and provide a more generic solution.

Which should pretty much wrap up this issue and that's why I allowed myself to mark it as Fixed for now, but feel free to re-open it, or post a new ticket, at any time if you have any further objections with the approach suggested in this ticket or related commit (we would surely be happy to hear your feedback).

Please let me know if you would have any further comments, feedback, questions, issues, objections, suggestions or concerns on the suggested feature request, this comment or this ticket in general, we would be glad to provide more information or explain in more details.

Many thanks to everyone for your great help, reviews, testing/reporting, feedback and comments on this issue.
Cheers!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.