Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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?
Comment | File | Size | Author |
---|---|---|---|
#42 | node_teaser.txt | 3.68 KB | gpk |
#38 | node_teaser_155337_5a.patch | 4.4 KB | gpk |
#20 | node_teaser_155337_5.patch | 4.98 KB | gpk |
#17 | node_teaser_155337.patch | 2.07 KB | Bevan |
#15 | node_teaser_155337_4.patch | 2.07 KB | gpk |
Comments
Comment #1
eli CreditAttribution: eli commentedHmm, that should read: node_teaser("<P>\nAnd a whole bunch of text............") returns "<P>"
Comment #2
Bevan CreditAttribution: Bevan commentedduplicate: http://drupal.org/node/180425
Comment #3
Bevan CreditAttribution: Bevan commentedGiles (gpk) pointed out that this isn't duplicate, but related.
Comment #4
gpk CreditAttribution: gpk commentedHere 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)
toif (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.
Comment #5
gpk CreditAttribution: gpk commentedComment #6
Bevan CreditAttribution: Bevan commentedI think this is a feature and is for drupal 7. Hmmm, not sure though.
Comment #7
eli CreditAttribution: eli commentedSeems like a bugfix for a current feature to me
Comment #8
Bevan CreditAttribution: Bevan commentedLooks 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
* 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.
Comment #9
Gábor HojtsyBevan: 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).
Comment #10
gpk CreditAttribution: gpk commentedThanks 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...
Comment #11
gpk CreditAttribution: gpk commentedFurther improvements to doc.
Comment #12
Bevan CreditAttribution: Bevan commented@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.
And a new line for this sentence.
Comment #13
gpk CreditAttribution: gpk commented>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?
Comment #14
Bevan CreditAttribution: Bevan commentedmeh, perhaps. I didn't think it was messy, but I guess that's subjective.
Comment #15
gpk CreditAttribution: gpk commentedHoping 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.
Comment #16
Bevan CreditAttribution: Bevan commentedSorry, I forgot to comment on this one last time;
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.
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.
Comment #17
Bevan CreditAttribution: Bevan commentedOr I could do it myself which would've been faster than writing that last sentence in the previous comment, and this comment... Meh
Comment #18
gpk CreditAttribution: gpk commentedNew patch works fine. RTBC.
Comment #19
Gábor HojtsyThanks, committed.
Comment #20
gpk CreditAttribution: gpk commentedNagyon 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).
Comment #21
Bevan CreditAttribution: Bevan commentedThis 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.
Comment #22
Mercury500 CreditAttribution: Mercury500 commentedI'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.
Comment #23
gpk CreditAttribution: gpk commentedHave 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
Comment #24
Mercury500 CreditAttribution: Mercury500 commentedGPK, 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.
Comment #25
Mercury500 CreditAttribution: Mercury500 commentedGPK, 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;"> </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> | <a href="http://www.diylife.com/forward/1050756/" title="Send this entry to a friend via email">Email this</a> | <a href="http://www.technorati.com/cosmos/search.html?rank=&fc=1&url=http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" title="Linking Blogs">Linking Blogs</a> | <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;"> </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> | <a href="http://www.diylife.com/forward/1050756/" title="Send this entry to a friend via email">Email this</a> | <a href="http://www.technorati.com/cosmos/search.html?rank=&fc=1&url=http://www.diylife.com/2007/11/30/pinecones-peanut-butter-make-yummy-bird-feeders/" title="Linking Blogs">Linking Blogs</a> | <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>
Comment #26
Bevan CreditAttribution: Bevan commentedDon't forget about
<!--break-->
Comment #27
gpk CreditAttribution: gpk commentedOK 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
Comment #28
Mercury500 CreditAttribution: Mercury500 commentedThanks 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
Comment #29
Mercury500 CreditAttribution: Mercury500 commentedThanks 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
Comment #30
gpk CreditAttribution: gpk commentedActually 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
Comment #31
Mercury500 CreditAttribution: Mercury500 commentedThanks 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:
and I'm guess I should try adding a 2nd break like this?
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
Comment #32
gpk CreditAttribution: gpk commentedIn 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]
Comment #33
Mercury500 CreditAttribution: Mercury500 commentedOops, 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
Comment #34
gpk CreditAttribution: gpk commentedSince 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.
Comment #35
drummThis has been noticed in 5.x, http://drupal.org/node/187010, so reopening.
Comment #36
drummExtra $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.
Comment #37
joep.hendrix CreditAttribution: joep.hendrix commentedPlease, get this fixed for D5!
Thanks!
Comment #38
gpk CreditAttribution: gpk commented$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 ...)
Comment #39
GreenJelly CreditAttribution: GreenJelly commentedhas 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.
Comment #40
gpk CreditAttribution: gpk commentedThis 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
within your node.tpl.php to see if anything is being added to the Body.
Comment #41
GreenJelly CreditAttribution: GreenJelly commentedthe 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
Comment #42
gpk CreditAttribution: gpk commented1. 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.
Comment #43
GreenJelly CreditAttribution: GreenJelly commentedI 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).
Comment #44
sinasalek CreditAttribution: sinasalek commentedNice patch thanks, tested in drupal 5.7 and works as expected.
Comment #45
drummLooks almost good, but
Should be using drupal_strlen() instead of strlen().
Comment #46
gpk CreditAttribution: gpk commented>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:
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?
Comment #47
GreenJelly CreditAttribution: GreenJelly commentedanyone got a dispute resolution process we can use?
Comment #48
gpk CreditAttribution: gpk commentedComment #49
MGParisi CreditAttribution: MGParisi commentedAs 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?
Comment #50
gpk CreditAttribution: gpk commentedOK 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).
Comment #51
drummCommitted to 5.x.
Comment #52
gpk CreditAttribution: gpk commentedCool, thanks drumm!
Comment #53
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #54
MGParisi CreditAttribution: MGParisi commentedDid #45 effect the patch? How do we apply this patch now?
Comment #55
gpk CreditAttribution: gpk commentedYou 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.