In weblinks_blocks_block_view() we add a css file:

function weblinks_blocks_block_view($delta, $options = array()) {
  drupal_add_css(drupal_get_path('module', 'weblinks_blocks') . '/weblinks_blocks.css');

However, it should be done using the #attached property. See also https://api.drupal.org/comment/44413#comment-44413

This issue was reported in #2429855: Blocks setting "Show description" not working as defined - credit to Gerhard for spotting the problem. It is unrelated to that problem so should be fixed separately, so that a proper record of the change can be made.

CommentFileSizeAuthor
#11 2443235_11.attach_block_css.patch5.64 KBjonathan1055
#1 2443235_1.attach_block_css.patch5.42 KBjonathan1055
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Status: Active » Needs review
FileSize
2443235_1.attach_block_css.patch5.42 KB

In weblinks_blocks_block_view() we currently put the generated content directly into $block['content'] in various place throughout the function, and doing it this way there is a default implication that this string is a markup value. However, to allow us to use the '#attached' property we need $block['content'] to be an array with key '#markup' holding the actual content. This requires minor changes to create $content instead of $block['content'] (and same with subject). Then only at the end, when we know there is some content, we can create the full $block array with the necessary keys.

As a side-note, we also used to have

drupal_add_css(drupal_get_path('module', 'weblinks') . '/weblinks.css');

in weblinks_blocks_form_alter() but that has since been removed, as it is done in the main weblinks.module code. Maybe we should review all the places where css files are loaded.

Here's a patch, including some commented-out debug which you may wish to use.

GStegemann’s picture

GStegemann CreditAttribution: GStegemann commented

I will test the patch later today.

As a side-note, we also used to have

Yes. See also #2457353: Web Links main Links page information is displayed twice.

GStegemann’s picture

GStegemann CreditAttribution: GStegemann commented

I have tested your patch right now.

It works so far that blocks will be displayed correctly. But when attempting to display a block on a Web Links page (via More link) only the word 'Array' is displayed:

<div class="weblinkCat weblinkCat-depth-0"><fieldset class="collapsible form-wrapper"><legend><span class="fieldset-legend">Beliebt Drupal Links</span></legend><div class="fieldset-wrapper">Array</div></fieldset>

It looks like that the interface between weblinks_blocks_block_view() and weblinks_page is broken.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Status: Needs review » Needs work

Yes you are right. It works fine for the term groups /weblinks/x but fails for the text strings such as /weblinks/recent and /weblinks/popular. I had forgotten that weblinks_page() for those urls re-uses the block output. The folloowing one-line change to add '#markup' fixes it.

    // Generate the subject and content by re-using our hook_block_view().
    $block = weblinks_blocks_block_view('weblink-' . $tid . '-' . $group_id, array('limit' => 0));

    $fieldset = array(
      '#title' => $block['subject'],
      '#value' => $block['content']['#markup'], // <<< this was just $block['content']
      '#attributes' => array('class' => array(
        variable_get('weblinks_collapsible', TRUE) ? 'collapsible' : '',
      )),
      '#children' => NULL,
    );

I would like expand the automated testing to trap this, as it was a major problem introduced here which I should have noticed.

GStegemann’s picture

GStegemann CreditAttribution: GStegemann commented

Still not good. Now just a < character is displayed:

<div class="weblinkCat weblinkCat-depth-0"><fieldset class="collapsible form-wrapper"><legend><span class="fieldset-legend">Beliebt Links</span></legend><div class="fieldset-wrapper"><</div></fieldset>

That's the content of #markup:

<div class="item-list"><ul class="weblinks"><li class="weblinks weblinks-item weblinks-link-37 first"><div class="weblinks-link"><a href="http://www.drupalcenter.de/" class="weblinks-extra weblinks-link previewlink" target="_blank" title="Drupal Center">Drupal Center</a></div><div class="description"><p>Deutschsprachige Drupal Community <br /> </p> </div><div class="weblinks-click-stats">Clicked 8 times. Last clicked 7. März 2015 - 17:29</div></li> <li class="weblinks weblinks-item weblinks-link-72"><div class="weblinks-link"><a href="http://www.drupal-initiative.de/" class="weblinks-extra weblinks-link previewlink" target="_blank" title="Drupal Initiative e.V.">Drupal Initiative e.V.</a></div><div class="description"><p>Die Drupal e. V. ist ein gemeinnütziger Verein, der dabei hilft, Drupal in Deutschland bekannter zu machen.</p> </div><div class="weblinks-click-stats">Clicked 3 times. Last clicked 13. Januar 2015 - 13:50</div></li> <li class="weblinks weblinks-item weblinks-link-36 last"><div class="weblinks-link"><a href="https://www.drupal.org/" class="weblinks-extra weblinks-link previewlink" target="_blank" title="Drupal.org">Drupal.org</a></div><div class="description"><p>Community plumbing</p> </div><div class="weblinks-click-stats">Clicked 2 times. Last clicked 12. Februar 2015 - 10:25</div></li> </ul></div>

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented

Odd. It worked for me ok. Must be something to do with putting the #markup directly into the output. I think we should not be dealing with direct html here, it feels like we are doing thing to manually.

GStegemann’s picture

GStegemann CreditAttribution: GStegemann commented

All-clear now. Somehow the patch was not fully applied. I guess due to all the patches the last days.

Sorry not catching this earlier.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Status: Needs work » Needs review

Now that I've added a couple of new tests in https://www.drupal.org/node/460622#comment-9767537 I'm going to re-queue thre patch, and it should fail.

jonathan1055 queued 1: 2443235_1.attach_block_css.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2443235_1.attach_block_css.patch, failed testing.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Status: Needs work » Needs review
FileSize
2443235_11.attach_block_css.patch5.64 KB

Here's the patch with #markup added (and some commented out debug removed). This should pass the new automated tests now.

GStegemann’s picture

GStegemann CreditAttribution: GStegemann commented
Status: Needs review » Reviewed & tested by the community

Thanks. Tested and works.

  • jonathan1055 committed c07e44f on 
    Issue #2443235 by jonathan1055: Add block css using the #attached...
jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Status: Reviewed & tested by the community » Fixed

Excellent, thank you.

Status: Fixed » Closed (fixed)

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