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.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna’s picture

Issue summary: View changes

Merged #1850402.

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
6.3 KB

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

FileSize
5.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

Status: Needs work » Needs review
FileSize
5.93 KB

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.