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.
Files: 
CommentFileSizeAuthor
#5 nodewords-n1850414-5.patch5.93 KBDamienMcKenna
PASSED: [[SimpleTest]]: [MySQL] 127 pass(es).
[ View ]
#3 nodewords-n1850414-3.patch5.93 KBDamienMcKenna
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/nodewords/nodewords.install.
[ View ]
#1 nodewords-n1850414-1.patch6.3 KBDamienMcKenna
PASSED: [[SimpleTest]]: [MySQL] 127 pass(es).
[ View ]

Comments

DamienMcKenna’s picture

Issue summary:View changes

Merged #1850402.

DamienMcKenna’s picture

Status:Active» Needs review
StatusFileSize
new6.3 KB
PASSED: [[SimpleTest]]: [MySQL] 127 pass(es).
[ View ]

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

DamienMcKenna’s picture

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

DamienMcKenna’s picture

StatusFileSize
new5.93 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/nodewords/nodewords.install.
[ View ]

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

Status:Needs work» Needs review
StatusFileSize
new5.93 KB
PASSED: [[SimpleTest]]: [MySQL] 127 pass(es).
[ View ]

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

DamienMcKenna’s picture

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

Issue summary:View changes

Added an item for moving the PHP filter higher up.