Here's an example node_teaser("

\nAnd a whole bunch of text............

") returns "

" as the teaser. This is espcially troubling since node_teaser also mostly ignores $format, so even though my input format doesn't break lines automatically (so \n should be irrelevant), node_teaser still gets tripped up.

Really though, I think the whole alogorithm needs a rethink. Perhaps a $min_length parameter would help?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eli’s picture

Hmm, that should read: node_teaser("<P>\nAnd a whole bunch of text............") returns "<P>"

Bevan’s picture

Version: 5.1 » 7.x-dev
Status: Active » Closed (duplicate)
Bevan’s picture

Status: Closed (duplicate) » Active

Giles (gpk) pointed out that this isn't duplicate, but related.

gpk’s picture

Version: 7.x-dev » 6.x-dev
FileSize
1.11 KB

Here is a simple patch against 6.x-dev that checks the input format and only interprets \n as a line break if the line break converter filter is present. The patch should help users of TinyMCE since TinyMCE produces HTML of the type posted by eli. Also the availability of the HTML corrector filter in Drupal 6 will help by closing a <p> element if node_teaser() splits it at the end of a sentence.

Note I have also taken the liberty of correcting the test for a "short body" if (strlen($body) < $size) to if (strlen($body) <= $size) since this is more logical and consistent. In practical terms one additional character is neither here nor there but it might make debugging less confusing in the future since the function will then behave more as expected. Also note that even at present node_teaser() can in certain circumstances return a teaser of length exactly equal to $size.

This issue is duplicated at http://drupal.org/node/165534.

gpk’s picture

Status: Active » Needs review
Bevan’s picture

I think this is a feature and is for drupal 7. Hmmm, not sure though.

eli’s picture

Seems like a bugfix for a current feature to me

Bevan’s picture

Looks great! I tested this by
* setting teaser length to 200 chars in post settings.
* disabling the 'line break converter filter' in the 'Full HTML' input format
* creating a node with content

testing
testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing 
testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing 

* seeing that the teaser for both input formats is one word 'testing'.
* applying the patch
* seeing that the teaser for the 'Full HTML' input format is more than one word; 'testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing testing'

I also ran gpk's test at http://drupal.org/node/180425#comment-634230 for regression-testing.

(We should turn that one into a unit test and contribute to simpletest.module. See http://www.lullabot.com/articles/introduction-unit-testing)

As for coding style, that looks fine to me. Perhaps add a line break before the comment to make it easier to read, but that's subjective.

Gábor Hojtsy’s picture

Bevan: you say this is ready for committ, except some coding style issue possibly? (BTW I'd wrap the two line comment earlier, so not only a single word remains on the second line).

gpk’s picture

FileSize
1.94 KB

Thanks for your comments Gábor and Bevan.

I've now added a bit more Doxygen documentation - particularly for $format, which needed further comment. Also I didn't like the syntax of the code I'd added - please try this version which I hope also reponds to other points raised. Have tested the patch and it appears to behave identically to the original.

Comments welcome as always...

gpk’s picture

Further improvements to doc.

Bevan’s picture

Status: Needs review » Needs work

@Gabor; The functionality (code changes) are fine and RTBC, we just need to finish the coding style and documentation tweaks

@gpk;

The functional code in your first patch is better because it doesn't duplicate the list of line-breaking html elements, which is a kind-of data source; '<br />' => 6, '<br>' => 4. This may seem trivial, but think about how someone editing this in the future will read and edit it.

The extra text is useful. These are just subjective grammar modifications, and a few extra words for extra clarification. My changes are in bold. Take this with a grain of salt and change only what you think is appropriate.

If the teaser length is not indicated using the special delimiter then
we try to end the teaser at a sensible place, like the end of a
paragraph, a line break or the end of a sentence (in that order of
preference)
.

And a new line for this sentence.

If the line break filter is present then we treat newlines embedded in
$body as line breaks.

gpk’s picture

>functional code in your first patch is better
Yes, but it seemed a rather messy way of doing something rather simple. Maybe it's the best we can do in PHP without being contrived?

Bevan’s picture

meh, perhaps. I didn't think it was messy, but I guess that's subjective.

gpk’s picture

Status: Needs work » Needs review
FileSize
2.07 KB

Hoping we are there ... finally ... nearly ...!!!

This differs from the original patch only in terms of comments/documentation. Have checked it still applies OK and has the desired effect.

Bevan’s picture

Status: Needs review » Reviewed & tested by the community

Sorry, I forgot to comment on this one last time;

If the end of the teaser is not indicated using the special delimiter

It's so small we may as well just put it in the comment. It's a fairly un-documented feature, so it makes sense to give it some more official documentation.

If the end of the teaser is not indicated using the <!--break--> delimiter

This is so trivial that I'm RTBCing anyway. if you get the patch in before Gabor gets to us, then great, otherwise I'm sure Gabor can make that change when he commits.

Bevan’s picture

FileSize
2.07 KB

Or I could do it myself which would've been faster than writing that last sentence in the previous comment, and this comment... Meh

gpk’s picture

New patch works fine. RTBC.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

gpk’s picture

Version: 6.x-dev » 5.x-dev
Status: Fixed » Needs review
FileSize
4.98 KB

Nagyon kösi, Gábor. Great to get this one fixed. (Applied to HEAD/drupal-6.x-dev in commit #89648.)

Unsurprisingly it still seems to be a recurring problem for D5 users (see e.g. http://drupal.org/node/165534#comment-642532, http://drupal.org/node/192615).

Therefore a patch for 5.x is attached. I have tested this on 5.1 but it should work on all 5.x (node_teaser() is unchanged in 5.x since the initial release of 5.0).

This 5.x patch gives node_teaser() identical functionality to 6.x - i.e. it also incorporates http://drupal.org/node/180425 and adds the $size parameter http://drupal.org/node/108456. The latter is an optional argument hence the new node_teaser() is still 100% compatible with 5.x. (It did not seem worth undoing the additional improvements to node_teaser() for the backported patch.) To make the 5.x version I just took the new 6.x version of node_teaser() and changed 'php/0' -> 'filter/1' (PHP filter) and 'filter/1' -> 'filter/2' (line break converter filter).

Bevan’s picture

Title: node_teaser sometimes generates bogus teasers » node_teaser generates bogus teasers (drupal 5)
Status: Needs review » Closed (won't fix)

This isn't critical enough for d5, but it's useful to leave the patch here so others can apply it if they feel the need.

Mercury500’s picture

I've been trying to resolve this problem for a while and just want to get this straight:

There is currently no way to display a certain pre-determined truncated amount of text for the Drupal "Teaser". ??

With (FULL HTML) FeedAPI content I can have - just the title, or the whole story. Any teaser setting other than unlimited results in no teaser at all. Views doesn't help with this either.

Is the node_teaser_155337_5.patch supposed to fix that? Cause I've installed it on a test 5.2 system and a 5.3 system and I see it doing nothing different than before.

I hope I have this wrong, so, is there a way to display truncated content without extensive hacking?

Thanks. This particular issue has taken up far too much time.

gpk’s picture

Have you tried setting the input format for a FeedAPI node to "Rich text"?

If this doesn't help then can you post (or attach) the raw HTML from a sample node here inside <code> tags so that markup and layout are preserved?

The idea was that this patch would fix this sort of problem so I'm keen to get to the bottom of it.

gpk

Mercury500’s picture

GPK, when you say "Rich Text" - where is this setting that are you referring? I have a FULL HTML radio box under FEEDAPI Settings. It's unchecked. That one?

Allowed HTML tags in FEEDAPI settings is similar to the "filtered HTML" input format under site configuration.

I have created a new input format for feedapi that allows most HTML, but tried removing the <p> tag to see if that issue was the cause. That didn't appear to make a difference.

This whole FEEDAPI settings seems to duplicate the input formats filters - I assume it's double filtering in some cases.

thanks.

Mercury500’s picture

GPK, here's some testing info with some wacky feed called Backyard Science :)

post settings - 1200 chars

input filter is standard "Filtered HTML" = <a> <em> <strong> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd> <img> and Lines and paragraphs break automatically.

FEEDAPI settings - Allow all HTML

Teaser displays this:

--------------------------------------------------------------

Pinecones+peanut butter make yummy bird feeders
Submitted by mercury on Fri, 11/30/2007 - 11:00.

Filed under: backyard science, food, kids, recreation, seasonal

--------------------------------------------------------------

Seems to cut off around the </p> tag. About 400 characters I believe.

Here's the BODY content from the node edit page:

<p>Filed under: <a href="http://www.diylife.com/category/backyard-science/" rel="tag">backyard science</a>, <a href="http://www.diylife.com/category/food/" rel="tag">food</a>, <a href="http://www.diylife.com/category/kids/" rel="tag">kids</a>, <a href="http://www.diylife.com/category/recreation/" rel="tag">recreation</a>, <a href="http://www.diylife.com/category/seasonal/" rel="tag">seasonal</a></p><img width="240" height="NaN" align="right" src="http://www.blogsmithmedia.com/www.diylife.com/media/2007/11/birdseed-feeder-by-diyalert.jpg" alt="pinecones, peanut butter, and bird seed make a bird feeder" />Since snow is now covering the ground where I live, I haven't seen too many birds. If I don't put out some special food for them, they will find somewhere else to find their food, and I won't be seeing much of them until next spring.<br /><br />You can <a href="http://www.diyalert.com/node/3996">make your own pinecone bird feeders</a> with pinecones, peanut butter, and bird seed. Slather some peanut butter on an open pinecone, roll it in bird seed, and allow the peanut butter to harden. Hang up in a tree outside. If you aren't so lucky to have a tree, you can throw them on the ground. Trust me, the birds will find them. Be careful if you buy pinecones at the craft store, as they may be preserved with fragrance oils that could harm the birds. If you don't have any pipe cleaners, you can use yarn or even fishing line to hang up the yummy bird treat.<br /><br />You can get the kids involved in this project. They will be proud of themselves, knowing that they are helping to feed creatures that otherwise might go south for the long (loooong) winter. What better way to spend a snow day than to watch the birds gobble up their man-made yummy treat!<br /><br />[via: <a href="http://www.craftzine.com/blog/archive/2007/11/how_to_make_pinecone_bird_feed.html?CMP=OTC-5JF307375954">Craftzine</a>]<p style="clear: both; padding: 8px 0 0 0; height: 2px; font-size: 1px; border: 0; margin: 0; padding: 0;">&nbsp;</p><p><a href="http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" rel="bookmark" title="Permanent link to this entry">Permalink</a>&nbsp;|&nbsp;<a href="http://www.diylife.com/forward/1050756/" title="Send this entry to a friend via email">Email this</a>&nbsp;|&nbsp;<a href="http://www.technorati.com/cosmos/search.html?rank=&amp;fc=1&amp;url=http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" title="Linking Blogs">Linking&nbsp;Blogs</a>&nbsp;|&nbsp;<a href="http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/#comments" title="View reader comments on this entry">Comments</a></p>

Here's the full RSS ITEM content (CDATA) fyi:

<item><title>Pinecones+peanut butter make yummy bird feeders</title><link>http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/</link><guid isPermaLink="true">http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/</guid><comments>http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/#comments</comments><description><![CDATA[<p>Filed under: <a href="http://www.diylife.com/category/backyard-science/" rel="tag">backyard science</a>, <a href="http://www.diylife.com/category/food/" rel="tag">food</a>, <a href="http://www.diylife.com/category/kids/" rel="tag">kids</a>, <a href="http://www.diylife.com/category/recreation/" rel="tag">recreation</a>, <a href="http://www.diylife.com/category/seasonal/" rel="tag">seasonal</a></p><img width="240" height="NaN" align="right" src="http://www.blogsmithmedia.com/www.diylife.com/media/2007/11/birdseed-feeder-by-diyalert.jpg" alt="pinecones, peanut butter, and bird seed make a bird feeder" />Since snow is now covering the ground where I live, I haven't seen too many birds. If I don't put out some special food for them, they will find somewhere else to find their food, and I won't be seeing much of them until next spring.<br /><br />You can <a href="http://www.diyalert.com/node/3996">make your own pinecone bird feeders</a> with pinecones, peanut butter, and bird seed. Slather some peanut butter on an open pinecone, roll it in bird seed, and allow the peanut butter to harden. Hang up in a tree outside. If you aren't so lucky to have a tree, you can throw them on the ground. Trust me, the birds will find them. Be careful if you buy pinecones at the craft store, as they may be preserved with fragrance oils that could harm the birds. If you don't have any pipe cleaners, you can use yarn or even fishing line to hang up the yummy bird treat.<br /><br />You can get the kids involved in this project. They will be proud of themselves, knowing that they are helping to feed creatures that otherwise might go south for the long (loooong) winter. What better way to spend a snow day than to watch the birds gobble up their man-made yummy treat!<br /><br />[via: <a href="http://www.craftzine.com/blog/archive/2007/11/how_to_make_pinecone_bird_feed.html?CMP=OTC-5JF307375954">Craftzine</a>]<p style="clear: both; padding: 8px 0 0 0; height: 2px; font-size: 1px; border: 0; margin: 0; padding: 0;">&nbsp;</p><p><a href="http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" rel="bookmark" title="Permanent link to this entry">Permalink</a>&nbsp;|&nbsp;<a href="http://www.diylife.com/forward/1050756/" title="Send this entry to a friend via email">Email this</a>&nbsp;|&nbsp;<a href="http://www.technorati.com/cosmos/search.html?rank=&amp;fc=1&amp;url=http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" title="Linking Blogs">Linking&nbsp;Blogs</a>&nbsp;|&nbsp;<a href="http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/#comments" title="View reader comments on this entry">Comments</a></p>]]></description><category>birds</category><category>food</category><category>peanut-butter</category><category>pine cone</category><category>PineCone</category><category>pinecones</category><category>seed</category><category>tree</category><category>winter</category><dc:creator>Anna Sattler</dc:creator><dc:date>2007-11-30T11:00:00+00:00</dc:date></item><item>

Bevan’s picture

Don't forget about <!--break-->

gpk’s picture

OK I think the node_teaser() function (http://api.drupal.org/api/function/node_teaser) is doing what it is supposed to in this instance, but unfortunately this is not what you want (the D6 version is largely the same as what you now have):

1. HTML tags are counted as part of the 1,200 "character" teaser length. This is a separate problem really though combines with other factors to make the teaser not behave as you would like.
2. As the documentation for the function describes, the teaser is not simply truncated at the length you specify but instead it attempts to end the teaser at a sensible place, within the length specified. Since it finds a complete <p> element within the node body it ends the teaser there in preference to a line break or the end of a sentence. The 2nd closing <p> tag doesn't occur until about 2,030 bytes into the node body. Although we've recently made some improvements to the function these enhancements won't make any difference in your case. Some of the functionality in the http://drupal.org/project/superteaser module does have a lot of merit, I think.

In your case it would probably all work better if the sequence <br /><br /> was treated as an "end of paragraph" by node_teaser(). In your revised version of the function, try changing the line

$break_points[] = array('</p>' => 0);

to read

$break_points[] = array('</p>' => 0, '<br /><br />' => 6);

Pls ignore my comments about "rich text" input format: i) I keep forgetting that there is no such thing in a default Drupal install ("rich text" input format is usually taken to mean an input format with *no* filters active), and ii) it won't help in your instance.

HTH

Mercury500’s picture

Thanks very much HTH, I'll try that.

In the past, I had found a Smarty modifier tag plugin for the previous CMS I was using called TRUNCATE_TAGSAFE (that also called another plugin called CLOSE_TAGS) that worked very nicely. It would truncate at the exact point specified, but then analyze and close any open html tags. Thus open H1 tags and stuff would not carry on and destroy all the formatting below it.

That would be nice, possibly optional, functionality for Drupal teasers!

best

m

Mercury500’s picture

Thanks very much HTH, I'll try that.

In the past, I had found a Smarty modifier tag plugin for the previous CMS I was using called TRUNCATE_TAGSAFE (that also called another plugin called CLOSE_TAGS) that worked very nicely. It would truncate at the exact point specified, but then analyze and close any open html tags. Thus open H1 tags and stuff would not carry on and destroy all the formatting below it.

That would be nice, possibly optional, functionality for Drupal teasers!

best

m

gpk’s picture

Actually that was me (HTH = hope this helps :-P).

Does the code tweak work?

For Drupal there is the htmlcorrector module http://drupal.org/project/htmlcorrector which does the tag closing business (it's in core in Drupal 6) and also clains to sort out nesting problems. This is probably worth using in any case. And I guess you could modify node_teaser() to do a straight truncate, but it is *supposed* to do something better than that! (Perhaps with a few more patches we can make it, finally, useful.) So do let us know how you are getting on ...

gpk

Mercury500’s picture

Thanks gain GPK.

So, what's interesting re this..there is no break_point in my node.module, but then I see that Drupal 6 is using break_point and Drupal 5 is using breakpoint. It's not exactly the same line regardless, so...

In the first instance of break_point (breakpoint) re the teaser is this:

  // In some cases, no delimiter has been specified. In this case, we try to
  // split at paragraph boundaries.
  $breakpoints = array('</p>' => 0, '<br />' => 6, '<br>' => 4, "\n" => 1);
  // We use strpos on the reversed needle and haystack for speed.
  foreach ($breakpoints as $point => $offset) {
    $length = strpos($reversed, strrev($point));
    if ($length !== FALSE) {
      $position = - $length - $offset;
      return ($position == 0) ? $teaser : substr($teaser, 0, $position);
    }
  }

and I'm guess I should try adding a 2nd break like this?

  // In some cases, no delimiter has been specified. In this case, we try to
  // split at paragraph boundaries.
  $breakpoints = array('</p>' => 0, '<br /><br />' => 6, '<br>' => 4, "\n" => 1);
  // We use strpos on the reversed needle and haystack for speed.
  foreach ($breakpoints as $point => $offset) {
    $length = strpos($reversed, strrev($point));
    if ($length !== FALSE) {
      $position = - $length - $offset;
      return ($position == 0) ? $teaser : substr($teaser, 0, $position);
    }
  }

m

UPDATE: Well this seems to work to some extent - depending on the how the source is formatted. So, it's much better.

Strangely though, now I'm getting lots of blank white pages (after saving or other input was sent) as I configure/change stuff. I've now removed that extra break and the white pages are gone. So, Drupal 5.3 doesn't like that tweak it appears. Might as well go to 5.5 this week...

m

gpk’s picture

In all versions of D5 (ie. including D5.5) the node_teaser() function is *not* updated (as you saw)! So you would first need to apply the patch at #20 (this makes it behave like the D6 version) and then tweak the code as outlined above at #27. If you think the patch should be included in any future releases of D5 then pls set the issue status to "patch (to be ported)".

[edit]
To understand why you were getting blank pages I'd need to see the raw HTML content of the node(s) in question and also be clear as to exactly what the source code of node_teaser() was at the time!
[/edit]

Mercury500’s picture

Oops, I see what happened. I had over-written the patched module.node with the original when the patch showed no difference last week. (I didn't realize the issue below while it was patched)

I have now used the patched version and made your tweak. Works great!

Except...the white page issue comes back. In administer, I can navigate and see various config screen, but if I change one, the next page is blank. I can backup and refresh manually and see that the change did indeed occur, but "refreshes after input" in the admin area are white blank pages.

Not wanting to blame the tweak, I went back to just the patched version of node.module = white pages.
Back to original 5.3 node.module and white pages disappear.

Given that I'm fairly new to Drupal, I tried several small changes on various admin settings to try to confirm.

I have no cache on.

"Seems" like that patch causes some admin settings refresh issue. At least in my install.

Here's my status info FYI:

Drupal 5.3
Configuration file Protected
Cron maintenance tasks Last run 1 day 18 hours ago
You can run cron manually.
Database schema Up to date
File system Writable (public download method)
MySQL database 5.0.45
PHP 5.2.4
SimplePie Parser Installed correctly
The current installed version of SimplePie is 1.0.1
Unicode library PHP Mbstring Extension
Web server Apache/1.3.33 (Unix)

m

gpk’s picture

Since you encounter the white screen problem with the patch, with the patch and the tweak, and also with a tweak but without the patch (#31), my hunch is that *any* change to node.module (and perhaps not just that file) would cause this problem. If you look at the modified version(s) of node.module you will see that only the function node_teaser() changes, and actually this function is only used in certain situations. The code would *not* be run when viewing or submitting an admin page AFAICS.

When you get the white screen, I presume that the server is returning empty content to the browser? Are there any messages in Drupal's "watchdog" log or in the webserver error logs? I wonder if it could be a problem with the PHP optimiser/code cache running on your server, but this is a guess really. Strange that the new code runs quite happily on your node pages when they are submitted.

drumm’s picture

Status: Closed (won't fix) » Needs review

This has been noticed in 5.x, http://drupal.org/node/187010, so reopening.

drumm’s picture

Status: Needs review » Needs work

Extra $size parameter should not be added to Drupal 5 since it is an API addition.

Otherwise, this is okay if it is reviewed and is otherwise the same as Drupal 6.

joep.hendrix’s picture

Please, get this fixed for D5!
Thanks!

gpk’s picture

Status: Needs work » Needs review
FileSize
4.4 KB

$size parameter removed.

Otherwise same as Drupal 6 version (note that as noted at http://drupal.org/node/155337#comment-643187 a couple of filters were renamed back to their D5 counterparts to get equivalent behaviour: changed 'php/0' -> 'filter/1' (PHP filter) and 'filter/1' -> 'filter/2' (line break converter filter).

Tested on a few pages, still seems to work as advertised (the important thing is to make sure the line break filter is *not* active, or you won't see it doing anything ...)

GreenJelly’s picture

has this patch been implemented in 5.7? If so, cck is not generating the $node->teaser element, and I wonder if it is a result of this code. I noticed that cck, puts a div wrapper around the body, and I believe this breaks the $node_teaser function from within cck. This is under the assumption that cck uses the node_teaser function, and that it is putting the wrapper around the output and not fckeditor. I have been trying to track down the culprit of this div tag from hell. It could be within og, cck, or fckeditor.

It should be said that no editor or function should apply an arbitrary div tag that wraps the body, however this is not the case.

gpk’s picture

This patch is still not committed to Drupal 5 - needs someone other than me to test it.

Suggest you post the raw HTML contents of the node Body field here, or in another topic and link to it from here, and I'll take a look. I'd be surprised if an extra <div> is being added round the Body before the teaser is generated but if you think it is then I suggest to have a look at the raw HTML directly from the {node_revisions} table in the database. Of course it may be being added programatically in which case it woudn't show up in the DB. If you are seeing a <div> round the Body when you view the page source this is more likely happening in themeing. You could also do

$n = node_load($node->nid);
print(htmlspecialchars($n->body, ENT_QUOTES));

within your node.tpl.php to see if anything is being added to the Body.

GreenJelly’s picture

the problem was that I was unkowingly posting html with a div tag on it... it was from the lorium ipsum generator... This makes me cautous because I can see allot of code that is wrapped in Div's (or any other tag)... but that is probably what a filter is for. If you know of one, shoot me over the address... As for testers on the 5.x code... paste the code in a file and I will test it (these patch files scare me) hehe

Thanks

gpk’s picture

FileSize
3.68 KB

1. If the <div> tag had had a line break after it before the "Lorem ..." and
2. if the length of the "Lorem ..." text exceeded the teaser length (trimmed post length) on the Post settings page

*then* the teaser will consist of an empty (and unclosed) <div> tag, since the teaser generation algorithm will unfortunately identify the line break as a suitable "break" point for the teaser.

It is normal for content to be heavily wrapped in <div> tags - this creates many opportunities for custom styling with CSS.

The patch only helps if the node's Input format does not include the line break filter. Applying the patch is the same as replacing function node_teaser() in modules/node/node.module with the attached version.

GreenJelly’s picture

I wrote a filter thats better then a line break filter... its a white space filter... I just haven't posted it to drupal.org... I think I disabled it because FCKeditor didn't appear to add any additional white spaces. But now that I think of it, I guess if you copy and paste HTML content, you get whitespace (tabs, spaces, line breaks, and carriage returns). Anyways, my filter replaces all white spaces with a single space... so five hundred spaces will become one. Not good on PHP because SQL statements and Variables may contain multiple whitespace (though I cant imagine why but it is a possibility).

sinasalek’s picture

Status: Needs review » Reviewed & tested by the community

Nice patch thanks, tested in drupal 5.7 and works as expected.

drumm’s picture

Status: Reviewed & tested by the community » Needs work

Looks almost good, but

  if (strlen($body) <= $size) {

Should be using drupal_strlen() instead of strlen().

gpk’s picture

>Should be using drupal_strlen() instead of strlen().
Yes (that fix got into D6 here http://drupal.org/node/204071), although node_teaser() should also, a couple of lines later, use drupal_substr() instead of truncate_utf8(), if it is to do what it says on the tin on the admin/content/node-settings page:

Length of trimmed posts:
The maximum number of characters used in the trimmed version of a post...

Note that in D6 truncate_utf8($string, $len, ...) was modified to take $len expressed in characters instead (D5) of bytes. In D6 it basically does a drupal_substr($string, 0, $len) anyway.

I kind of assume that fixing this underlying problem (by using drupal_substr() instead of truncate_utf8()) would be considered undesirable at this stage in the lifecycle of D5 since it would cause a change in behaviour for existing D5 sites, which in some languages could result in inconsistent teaser lengths and confused site admins if/when they update.

Also, correcting the substr() without changing the truncate_utf8() would cause erratic teaser lengths in languages that mainly or wholly use multibyte characters since a node Body that just fails to fit in the $size limit would then be severely truncated by the truncate_utf8() even *before* the "breakpoint" search algorithm in node_teaser() kicks in.

So on balance I think the patch at http://drupal.org/node/155337#comment-711739 is still the best overall "solution".

Does this reasoning hang together?

GreenJelly’s picture

anyone got a dispute resolution process we can use?

gpk’s picture

Status: Needs work » Needs review
MGParisi’s picture

As far as the code presented here, I have been using this for sometime without issue. I pair it up with whitespace filter, Is the new patch at the comment together work better?

gpk’s picture

Status: Needs review » Reviewed & tested by the community

OK I should probably have restored this to RTBC at #46 in response to drumm@#45. D5 patch remains here http://drupal.org/node/155337#comment-711739, between #37 and #38 :P

Happy to reroll if my arguments at #46 don't hang together.

Motivation for changing status was that on my first visit to IRC for ages this came up again (http://www.disobey.com/bot/log/drupal-support/2008-05-30#T701408 and scroll up to clear the sticky header).

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 5.x.

gpk’s picture

Cool, thanks drumm!

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

MGParisi’s picture

Did #45 effect the patch? How do we apply this patch now?

gpk’s picture

You can ignore #45. node_teaser() still uses strlen in 5.x-dev.

2 ways to "apply" the patch:

1. upgrade to the current 5.x-dev (5.8-dev) ... the most recent patch to this was on 23 June http://drupal.org/project/cvs/3060?branch=DRUPAL-5 so there's probably not a hidden critical bug waiting there to catch you!. In due course you could upgrade to 5.8, as and when formally released.

2. manually apply the patch attached here http://drupal.org/node/155337#comment-711739.