Fix a serious bug and clean up teaser generation

-1 on using the node reference; some node types need to extract the teaser from a field or collection of fields not named "body."

The other patch looks fine to me, but I haven't tried running it.

Not entirely. It seems sensible to me for a module to concatenate two fields and run them, as a unit, through node_teaser() to generate a teaser that way. Using the node reference, they would have to abuse the body field for this (and if one of the two fields in question is "body" already, that's a sticky situation).

Can't this be solved by allowing the passing of NULL as the $format, and to disable this check in that case? I really like this solution, it's much better than the current hack.

I was talking about the alternate solution by the way.

I wonder if this is the best way to check for the PHP code filter. It's not exactly clean code. Except for that check, this patch looks good.

But what do we do when I create a filter X which needs special treatment just like the PHP filter?

+1 for this patch.

As for arbitrary filters that may require special handling, that problem existed before this patch existed. My thought on solving it would be to move teaser generation to occur on the filtered text. This would solve the problem of needing to deal with special-cases filters like the PHP code filter and any others that will encounter problems if their input is truncated. Of course, moving teaser generation in this way will cause it to potentially generate unclosed tags which won't be fixable by an htmlcorrector filter because the filters would have already done their processing. Of course, if htmlcorrector were part of core, we could choose to run it after the teaser generation rather than with the other filters.

Anyway, those are just thoughts... not necessarily "good ideas." :-) I suggest we commit this patch as-is to deal with the PHP code filter as a special case, and deal with allowing other filters to be special cases later. As far as I know, no other filter needs special handling at the moment other than the PHP filter.

Generating the teaser from the raw input isn't going to work; this is illustrated by the PHP filter, but I could come up with a handful of other examples. The proposed patch is a quick hack; it fails to address the larger problem.

Generating the teaser from the output (filtered input) may result in unexpected behavior, but at least it isn't a hack. Generating the teaser from the output would work if the filter system would not cache all generated output. I'd have to dig the code but if I remember correctly, this was within the filter system's capabilities. For example, how does it deal with 'non-teaser dynamic output'?

I'd certainly prefer to explore the 'on output' path.

Goba, not quite. I suggest to use 'on output teasers' in combination with _not_ caching dynamic teasers. Like that, there won't be unexpected behavior because PHP-based teasers wouldn't get cached. This should be fairly easy to accomplish because the filter system supports this already. Open filter.module and search for 'no cache'. Makes sense?

Goba: most teasers would get cached except those that are specified to be dynamic (i.e. those who are generated by a filter with the 'no cache' attribute set). The current filter system only caches filtered output if it can be cached. For example, it does not cache filtered output that is the result of the PHP filter. This shouldn't (and wouldn't) be any different for teasers. See filter_format_allowcache() and check_output() for details.

Either way, I'll give it some more thought. Like, I want to investigate how this affects teasers being stored in the database.

I'm definitely in Goba's camp. Yes we would be dealing with the PHP filter as a special case, but then it is a really special filter. In fact, it is the only filter like it: it does not transform user-input from one format to the other, it executes it. It is itself a hack, but one which works very well within the context of the filter system.

By generating teasers and caching them, we would need to regenerate all teasers if filter settings change. This isn't a problem in Dries' situation, but it is not clear to me how his approach would deal with storing teasers in the database. We also need to consider how this would mesh with custom teasers, like flexinode.module or excerpt.module.

Using PHP code in Drupal core is only going to increase when we're going to collect code snippets in the handbook. I don't think treating the PHP filter as a special case is unacceptable for core: until we see another code evaluator filter, this whole discussion is unnecessary.

There is another thing to consider: the only reason we can short-cirtcuit teaser generation for PHP code is because you can decide yourself in the PHP code whether to output a teaser or the full body. This is not something you can do when the "no cache" filter in question is a 'normal' filter without conditional logic in it.

The current filter system works the way it does because people currently want it to do all those things. Custom evaluation filters is not one of those things.

The fact is, this patch improves PHP teaser behaviour massively already. What we have now in core is an even bigger hack. +1 on this patch, for 4.6 final.

I think there was a bug in Goba's patch. strpos(...) != false should be strpos(...) !== false. strpos() can return 0 when the string is matched at the beginning.

New patch for CVS, should apply to 4.6 too.

I convinced Dries recently that this patch is still a good idea. Generating teasers dynamically is something which we /may/ do in the future, but until someone actually asks for that, this solution is much better than what we had before.

Commited to 4.6 and HEAD.