There are problems in nodewords_metatag_from_node_content():

  • Bug: The strip_tags() call is completely ignored.
  • Task: The code is confused by use of $text when the $result variable already exists.
  • Task: The _nodewords_teaser_match_callback() could be just merged inline.
  • Feature: It doesn't IMHO make sense that the alt tags of an image be added inline, possibly within a sentence, there should at least be a way of customizing it so that e.g. it's wrapped in parenthesis.
  • Task: In the interest of code simplicity, each filter call should be separate rather than stacked.
  • Task: There's no way of customizing a global regex for filtering node teaser output, so there's no point in keeping the code in nodewords_metatag_from_node_content().
  • Task: Move the php filter check higher up in the function to avoid wasting processing time.
CommentFileSizeAuthor
#5 nodewords-n1850414-5.patch5.93 KBdamienmckenna
#3 nodewords-n1850414-3.patch5.93 KBdamienmckenna
#1 nodewords-n1850414-1.patch6.3 KBdamienmckenna

Comments

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes

Merged #1850402.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review
StatusFileSize
new nodewords-n1850414-1.patch6.3 KB

This patch resolves the issues noted above, along with another few improvements for code style, comments, etc.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I need to test that the regex functionality still works, then I'll commit it.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new nodewords-n1850414-3.patch5.93 KB

A slight rearranging of the code, moved the php-check slightly higher up.

Status: Needs review » Needs work

The last submitted patch, nodewords-n1850414-3.patch, failed testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new nodewords-n1850414-5.patch5.93 KB

Forgot to rename a hook_update_N() call after another update was added elsewhere.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed

After some further testing I think this is fine - it doesn't really change anything (other than wrapping the alt text in parenthesis), just cleans up the code and fixes a typo introduced in 1.13. Committed.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Added an item for moving the PHP filter higher up.