Blocks setting "Show description" not working as defined

Regarding the limit I think it is working ok, at least it is corect for other blocks. The Random one is slightly different in that the content is not refreshed until the time is up, so it may not relect your desired setting until the next rebuild.

Thanks for noticing the css attachment. Yes, we'll do that.

Done some investigation and I've found that the teaser display problem is created in function theme_weblinks_block_item() where we derive our own summary value:

  $items = field_get_items('node', $node, 'body');
  $node_format = $items[0]['format'];
  switch ($desc) {
    case 'none':
      $description = NULL;
      break;
    case 'teaser':
      $description = '<div class="description">'
                   . check_markup(text_summary($items[0]['value'], $node_format),
                                  $node_format, $langcode = NULL /* TODO Set this variable. */, FALSE)
                   . '</div>';
      break;
    case 'body':
      $description = '<div class="description">'
                   . check_markup($items[0]['value'], $node_format, $langcode = NULL /* TODO Set this variable. */, FALSE)
                   . '</div>';
      break;
  }

This probably worked in Drupal6 but in D7 if a separate summary value has been entered for the node there is a 'summary' key value in the $items[0] array. Hence we should check if this exists and use it as the input to text_summary in preference to $items[0]['value'].

Function text_summary() can take a third parameter which is the trim length. The default is taken from variable 'teaser_length' which seems to be maintained for backwards compatibility but is no longer used, reading through the update functions in node.install. The old default was 600 but this is no longer configurable like it was in D6. It must be in the field config settings somewhere. Anyway, that's half of the problem discovered.

I think the Random block behaves in the same way as the others - for testing it is better to have the 'update every' value blank so that the content is refreshed as soon as the settings are changed.

Here's a patch which addresses the teaser problem. The trim length is recovered using

        $body_info = field_info_instance('node', 'body', 'weblinks');

I have also done the work on the css #attached problem, but I think it is better to review these two things separately. Although they do not really affect each other, it just makes the patch look more complex. Two of the changes in this patch are to split up creating arrays and returning them. This is better practice as it allows debugging to check the values before return.

I have created a separate issue #2443235: Add block css using the #attached property

Thanks for drawing that to my attention. OK, comparing the two implementation, the differences are:

1. In weblinks_weblinks_preprocess() we use $field_langcode = field_language('node', $node, 'body' which is good. This is marked as a @TODO in theme_weblinks_block_item() so I could add that to the patch
2. weblinks_weblinks_preprocess() caters for no 'body' field. Do we really need to allow for this?
3. weblinks_weblinks_preprocess() uses _text_sanitize() which I've not used before but it looks better than always calling check_markup() because it conditionally calls check_markup() or check_plain(). I could change the Blocks version to use this
4. weblinks_weblinks_preprocess() uses the entire summary if that field exists, and does not trim it at the shorter length. I think this is better and I could change the Blocks version to do likewise.

I also noticed that in weblinks_weblinks_preprocess() the variable $sanitize is only set where we have a body field, which is fine. But given that it is always 1 the variable is never going to be used when it is not 1, so it can be removed, and the following code simplified by removing the conditional tests.

Here is a patch (with debug) to show my work so far.
1. I have added the $langcode support
2. I've catered for the body field being deleted (but not actually tested this yet)
3. I have used _text_sanitize()
4. Always trim the text for teaser, even if using the separte summary field. This involved making a temporary array to pass to _text_sanitize() but it works well.

Maybe some more work to do, but thought you should see (and check) the direction of the work so far.

That was an error, sorry! I had previously added the lines

  if ($description) {
    $description = '<div class="description">' . $description . '</div>';
  }

after the switch, so that if we need to adjust the html or classes then it is is all done in one place, to be consistent. I had some editting problems yesterday and those lines got deleted by mistake. Here's an updated patch.

Yes I know that the styling is untidy - we discovered that in #2406139: Last click time and click count doubled or trebled up in title and it is going to be fixed in that issue as mentioned here

I've tested the patch now. I think it works.

Based on your comment, are you happy if I commit the attached patch? It is the same apart from all debug removed and the output returned via variable $output not directly in the return statement.

Thank you for your testing. We are making really good progress now.