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.
Members fund testing for the Drupal project. Drupal Association Learn more

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.