Download & Extend

Calculation of $node->readmore seems a bit flawed (sets to TRUE when it shouldn't be)

Project:Drupal core
Version:6.x-dev
Component:node system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (duplicate)

Issue Summary

In node.module, in the function node_prepare (around line 520), $node->readmore is often set to TRUE incorrectly.

The function sets readmore based on a strlen comparison between $node->teaser and $node->body. The problem enters if $node->teaser doesn't exist (or is set to an empty string) when teasers are disabled. The simple fix is to prepend one or both of the logical tests shown below:

Original:

<?php
 
function node_prepare($node, $teaser = FALSE) {
   
$node->readmore = (strlen($node->teaser) < strlen($node->body));
?>

Fixed:

<?php
 
function node_prepare($node, $teaser = FALSE) {
   
$node->readmore = $teaser && (strlen($node->teaser)) && (strlen($node->teaser) < strlen($node->body));
?>

Thoughts?

Comments

#1

Status:active» needs review

Making your suggestions into a patch and changing the status so it lands in the patch queue.

AttachmentSizeStatusTest resultOperations
34135.patch01588 bytesIgnored: Check issue status.NoneNone

#2

Status:needs review» needs work

looks good. however, patches must be against HEAD. then we decide if backport is worthwhile.

#3

Version:4.6.3» 4.6.9

#4

Version:4.6.9» 4.7.3
Status:needs work» needs review

Patches for 4.6.9 and 4.7.3

AttachmentSizeStatusTest resultOperations
node.module.patch.tar.gz489 bytesIgnored: Check issue status.NoneNone

#5

Version:4.7.3» x.y.z

Rerolled for CVS HEAD

regards,
--AjK

AttachmentSizeStatusTest resultOperations
node.module_1.698_p01.txt521 bytesIgnored: Check issue status.NoneNone

#6

patch created in "the proper place" (drupal root path).

regards,
--AjK

AttachmentSizeStatusTest resultOperations
node.module_1.698_p02.txt560 bytesIgnored: Check issue status.NoneNone

#7

Mmm, odd. How to reproduce/visualize this problem?

#8

duplicate issue http://drupal.org/node/82070 seems to show how to reproduce in a simple way so it demostrates it's been a problem for two developers at least (I marked that issue as dup pointing it to this, older, issue)

regards,
--AjK

#9

This bug also affects the views.module -- see http://drupal.org/node/81360

#10

Version:x.y.z» 6.x-dev
Status:needs review» needs work

patching file modules/node/node.module
Hunk #1 FAILED at 677.
1 out of 1 hunk FAILED -- saving rejects to file modules/node/node.module.rej

#11

I am curious, why has this been set to 6.x-dev ?

Regards,

Caroline

#12

I think the test should be something like this:

<?php
$node
->readmore = $teaser && ($node->teaser !== '') && ($node->teaser !== $node->body);
?>

We shouldn't assume that the teaser is embedded in (or a subset of) the body, which is what the original logic does. If they are different (via nodeteaser.module), then we should display readmore even if the body is shorter than the teaser (e.g. maybe it has an embedded video or image that doesn't take much markup).

#13

I don' t know what the status of this issue is, it doesn't look like it's been worked on for a while. I've created a new patch. Please test it!
I used the following code:

<?php
 
function node_prepare($node, $teaser = FALSE) {
   
$node->readmore = $teaser && (strlen($node->teaser)) && (strlen($node->teaser) < strlen($node->body));
?>

I've tested it on my macchine with views, and it seems to work.

AttachmentSizeStatusTest resultOperations
nodereadmore.patch858 bytesIgnored: Check issue status.NoneNone

#14

if you've patched against head you should change the Status to patch (code needs review)

#15

Status:needs work» needs review

Patch rolled against HEAD using the logic in #12. It would be nice if this could get in since it is a problem for certain views and other things.

AttachmentSizeStatusTest resultOperations
readmore.patch839 bytesIgnored: Check issue status.NoneNone

#16

I won't be able to test this for a while. Does this prevent a "read more" when there is a <!--break--> at the very end of the text? I have some nodes that are very close to the teaser setting, so I go ahead and put this in rather than changing the setting for everything.

I reported this and provided a patch in http://drupal.org/node/148323.

#17

This still applies with offset, code looks cleaner, but as with NancyW I'm not sure exactly how the behaviour is changed.

#18

Added trimming to avoid extra line breaks in $node->body when comparing.

AttachmentSizeStatusTest resultOperations
node.module-34135.patch867 bytesIgnored: Check issue status.NoneNone

#19

Status:needs review» closed (duplicate)

Marking as duplicate of #201854: "Read more" link sometimes shown when it shouldn't be, or not shown when it should since the other issue has more relevant discussion and better patch for 6.x/7.x.

nobody click here