Issue status update for http://drupal.org/node/18836 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Goba Reported by: Goba Updated by: Goba Status: patch Node module will not know anything about arbitrary filters, but it is not supposed to know anything about them either... We can put some new shiny hook in here to let filters help doing teasers, but I think it is just overcomplicating the question here. Sure this patch will not solve issues with arbitrary filters, this just solves an existing bug, and cleans up code. I don't have a good idea though to handle arbitrary filters while generating the teaser, and would really like to get the fix into Drupal for 4.6.0. Any good ideas are welcome! Goba Previous comments: ------------------------------------------------------------------------ March 13, 2005 - 15:37 : Goba Attachment: http://drupal.org/files/issues/Drupal.teaser-bugfix-and-cleanup.patch (4.48 KB) The node teaser generator keeps the whole body as the teaser, if there is indication that PHP code is included in the body (since it is unkown, what is going to come out of the PHP parser). The only serious problem is that it does not check if the PHP filter is included in the input format, so it does keep the complete body even if there is no PHP parsing on the code. This breaks PHP code examples in articles or blog posts, which are not supposed to be parsed, but which require proper teasers. The solution is to make the teaser generation code aware of the input format (pass the whole node), and let it provide the teaser that way, depending on the input format. The attached patch also cleans up that very unprofessionaly looking sequence of copy-pasted stuff in the node_teaser function. ------------------------------------------------------------------------ March 13, 2005 - 15:50 : Goba Attachment: http://drupal.org/files/issues/Drupal.teaser-bugfix-and-cleanup-alternate.pa... (4.07 KB) Here is an alternate approach, which does not take the node as reference, but asks for the body and the format, and then returns the teaser, as before. Pick this or the previous one, or suggest something else :) ------------------------------------------------------------------------ March 17, 2005 - 13:59 : Goba Is someone willing to review these fine patches? Either one :) ------------------------------------------------------------------------ March 17, 2005 - 14:57 : JonBob -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. ------------------------------------------------------------------------ March 17, 2005 - 16:45 : Goba JonBob, this is just an internal decision, as node_teaser() is not a hook implemented, it is just the default logic used to generate the teaser. Other modules are free to provide a teaser for a node in the node validation process before this logic is invoked, so that the custom teaser is not overwritten. Passing on the node reference or the body and the format is just a matter of programming style... ------------------------------------------------------------------------ March 17, 2005 - 16:49 : JonBob 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). ------------------------------------------------------------------------ March 17, 2005 - 16:51 : Goba Ah, understood, and agreed! ------------------------------------------------------------------------ March 18, 2005 - 09:34 : Steven 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. ------------------------------------------------------------------------ March 18, 2005 - 09:35 : Steven I was talking about the alternate solution by the way. ------------------------------------------------------------------------ March 18, 2005 - 19:46 : Goba Attachment: http://drupal.org/files/issues/Drupal.teaser-bugfix-and-cleanup-alternate2.p... (4.1 KB) Steven's suggestion implemented. ------------------------------------------------------------------------ March 20, 2005 - 15:14 : Goba Seems like everyone is happy with this patch :) Or is that only my single minded view? ;) ------------------------------------------------------------------------ March 20, 2005 - 20:08 : Dries 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. ------------------------------------------------------------------------ March 20, 2005 - 20:15 : Goba Dries, the filter format API provides filter_list_format() which returns an array in this format. I don't think that redefining the filter API is a good move in this part of the release cycle. ------------------------------------------------------------------------ March 21, 2005 - 22:16 : Dries But what do we do when I create a filter X which needs special treatment just like the PHP filter?