Read More link needs to insert itself before more tags that could end a teaser

This is the code I used in #16161: Move Read More link to end of node content that seemed to work pretty well. Maybe it can help here somehow.

  // This is a list of all the block tags, taken from _filter_autop().
  $block_tags = '(?:table|thead|tfoot|caption|colgroup|tbody|tr|td|th|div|dl|dd|dt|ul|ol|li|pre|select|form|blockquote|address|p|h[1-6]|hr)';

  // Get last position of the last closing marker and if found, insert the link
  // before the marker, otherwise simply append link to teaser.
  if (preg_match('!</?' . $block_tags . '[^>]*>$!i', $teaser, $match, PREG_OFFSET_CAPTURE)) {
    $teaser = substr_replace($teaser, $link, $match[0][1], 0);
  }
  else {
    $teaser .= $link;
  }

I completely agree that the module needs to handle better link insertion. While Dave Reid's code above seems to be the answer to this problem, we will need to drastically reduce the number of allowed block-level elements into which a "Read more" link can be inserted. The majority of the elements Dave lists shouldn't have anything inserted before its closing tag for two reasons: (1) semantic tags like <address> will contain unsemantic data, and (2) some elements will break or will be rendered invalid.

Take the <ul> element, for example. Let's say a node's teaser ends like this:

<ul>
  <li>My list item #1</li>
  <li>My list item #2</li>
  <li>My list item #3</li>
</ul>

Using Dave's code, the "Read more" link would be inserted before the closing <ul> tag:

<ul>
  <li>My list item #1</li>
  <li>My list item #2</li>
  <li>My list item #3</li>
  <span class="read-more"><a href="/node/1">Read more</a></span>
</ul>

This is invalid markup.

Here are the block-level elements I have determined are allowed to accept inserted content:

• <p>
• <div>

I am skeptical that the following should have "Read more" links inserted into them, as they may be unsemantic:

• <blockquote>

I have determined the following should not accept inserts:

• <table>
• <thead>
• <tfoot>
• <caption>
• <colgroup>
• <tbody>
• <tr>
• <td>
• <th>
• <dl>
• <dt>
• <dd>
• <ul>
• <ol>
• <li>
• <pre>
• <select>
• <form>
• <address>
• <hr>
• <h1> through <h6>

I need feedback! Please let me know if there are others we should consider.

It wouldn't be that hard to modify the code in #1 to make sure it is added after the closing tag if needed.

Dave: AFAIK, the module already performs the behavior you've described. If an "approved" element -- for lack of a better term -- isn't found, it simply appends <span class="read-more"><a href="/">Read more</a></span> to the end of the teaser.

You have a better understanding of the code you suggested in this thread than I, so please correct me if I'm wrong. This issue is just slightly outside of my comfort zone, so I'm relying on your code and the input of others to resolve it.

I have implemented Dave's method of parsing the teaser and inserting the link. I limited the block-level elements to only those that made semantic sense: <p>, <blockquote>, and <div>.

Changed have been committed to 6.x-3.x-dev. It will need some testing before it's ready to release. Even though there's no real patch, I'll mark this issue "patch (code needs review)" to encourage people to check it out.

#16161: Move Read More link to end of node content has been marked as a duplicate of this issue. I am changing the title of this issue to resemble the duplicate a bit more.

Here's a comment from David_Rothstein in the duplicate issue mentioned above (#7):

1. It sort of "feels" to me like the decision of where to put the read more link belongs in the theme layer - for example, see http://api.drupal.org/api/function/template_preprocess_node/7 and how it splits off the taxonomy links. I wonder if there's a way to do this one similarly?

2. Does it really have to be the job of this patch to figure out how to deal with HTML that gets cut off between tags? I'm not sure it does. Drupal has an HTML Corrector filter that serves this purpose, and the "read more" link should append perfectly fine to any content that is rendered using that filter. If someone is using a text format that doesn't contain that filter, by contrast, then they are already going to have all sorts of problems with HTML getting chopped off and rendered in weird ways, regardless of whether the read more link is in there or not.

3. Check out #372743: Body and teaser as fields for an interesting issue that might be related to this one. If the node body just becomes a regular old field, and if any textfield in Drupal can be "teaser-enabled", then maybe the 'read more' link is something that gets appended on a field-by-field basis? That's probably thinking a bit ahead, though :)

@David_Rothstein: Have you reviewed the most recent commits to the 6.x-3.x-dev branch? If you could review the code, I'd love to hear your thoughts. I think my changes address some of your general concerns.

Regarding #1: I agree that the Read More link could belong in the theme layer. However, I don't see how the template_preprocess_node() function relates to the purpose of this module, which specifically moves the Read More link from the "node links" area to the end of the node teaser's content. Please explain further if I'm not understanding your point.

Regarding #2: This is a very interesting question. However, whether the Read More link is added prior to or after the HTML Corrector filter is run is somewhat irrelevant. If the teaser ends in the middle of a list, the Read More link still doesn't make sense if it's tacked onto the end of the last list item (i.e., inside the <li> element). Here's an example. Which makes more sense to you?

Link placement scenario A: inside the <li> element:

This is a node that contains a list. Check it out here:

• This is my list. I am the first item.
• I am the second item.
• I am the third item. There's a fourth. Read More »

Link placement scenario B: outside the <li> element:

This is a node that contains a list. Check it out here:

• This is my list. I am the first item.
• I am the second item.
• I am the third item. There's a fourth.

Read More »

Scenario B makes much more sense to me. I don't think the Read More link should be confused as a part of the list.

Now, I'm not sure this actually answers your question! I'd like to hear more about what your suggesting to make sure I fully understand you.

Regarding #3: Unfortunately, Drupal 7.x's adoption of fields in core doesn't help us here. It's a HUGE change, and it can't be backported to 6.x in any analogous way.

I agree that "Link placement scenario B" is the best, but wouldn't that happen automatically when the HTML Corrector filter is enabled? The filter would close the <li> and <ul> tags for you, and then the read more link would get appended after that. It's only for tags where you want to specifically insert it inline that you need to add extra code -- and it seems that this code already exists above (I think I didn't quite realize that in the other issue). I agree that <p>, <div> and <blockquote> seem like the right tags to do that with.

My comments regarding the template_preprocess_node() function and fields were specifically about putting this feature in D7 core -- so not relevant for the D6 module :)

@David_Rothstein: I see your point regarding the HTML Corrector, but I'd rather not assume (or require) the user has it enabled to properly use this module. I will look into adding a conditional that checks for the filter before falling back to the module's behavior.

Whatever the case, I'd like to address the HTML Corrector filter in a separate issue.

Regarding the "approved" tags (<p>, <div>, and <blockquote>), I decided to remove <blockquote> from the list. Adding the link to <blockquote> is a bit confusing, as it alters the content of the original quote. One may also think the quote originally included a "Read More" link.

So... All of that aside, I'd love to get some feedback on the latest 6.x-3.x-dev commits if possible. Marking this issue fixed would bring us one step closer to a new release that addresses some major changes and fixes.

After much, much testing, it's clear that only the <p> tag is appropriate. We can't insert the link before a closing <div> because it interferes with CCK field output.

I'm marking this issue "won't fix," as we arrived at the same conclusion we started with.