Contrib.social

Using saveHTML() breaks other links

Open on Drupal.org →
Created on 7 February 2023, over 1 year ago
Updated 14 July 2023, 12 months ago

Problem/Motivation

I just recently updated from 1.3 to 3.0.0-beta2 and that broke several links in my editor.
I use a custom module that makes it possible for the users to add media over the Linkit interface by referencing it like this: [media/file/1].

Normally this would look like this before the [media/file/1] gets replaced by the actual path:
<a href="[media/file/1]">[media/file/1]</a>

But after it went through the filter_spamspan filter, it looks like this:
<a href="%5Bmedia/file/1%5D">[media/file/1]</a>
The brakets get converted by SpamSpan to %5D and %5B. My problem is that the filter changes that before I replace the [media/file/1] with the actual path to the file.

I was able to find out where the [] get converted to %5D and %5B. This happens in the function SpamspanDomTrait::toStringHtmlDocument(\DOMDocument $document) in line 129:
return implode('', array_map([$document, 'saveHTML'], iterator_to_array($div->childNodes)));

I know that this is a very specific use case but I was wondering why the module does that to other links that are not an email address. Shouldn't only email addresses be affected by this filter?

🐛 Bug report
Status

Fixed

Version

3.0

Component

Code

Created by

🇩🇪Germany lmoeni

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @lmoeni
  • Comment over 1 year ago →
    🇷🇴Romania vitalie

    Hi @lmoeni,

    Could you check if the order of the filters is such that spamspan filter runs after linkit filter?

  • Comment over 1 year ago →
    🇩🇪Germany lmoeni

    I checked and the linkit filter runs before the spamspan filter.

  • Comment over 1 year ago →
    System Message
  • Comment over 1 year ago →
    🇷🇴Romania vitalie

    Changed saveHTML to saveXML.

  • Status changed to Fixed over 1 year ago
  • Comment over 1 year ago →
    🇷🇴Romania vitalie
  • Comment about 1 year ago →
    System Message

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

  • Status changed to Fixed 12 months ago
  • Comment 12 months ago →
    🇩🇪Germany Grevil

    This fix created some MAJOR regression and breaks markup under certain circumstances! See 🐛 SpamSpan breaks markup Fixed .

    Of course, spamspan shouldn't touch links as mentioned above, but the applied fix is incorrect nonetheless. This will get reverted and further looked into in 🐛 SpamSpan breaks markup Fixed .

  • Comment 12 months ago →
    🇩🇪Germany Anybody Porta Westfalica

    @vitalie this wasn't really a nice change into dev. No tests were added, no reviews by other maintainers... shouldn't happen again please for such a heavy change, even while being "just another method".

    XML is quite different from HTML and by this commit directly into the main branch you put this into the next release... uncool. Anyway thank you for trying to fix this.

    @lmoeni: Please re-check your filter order or the markup. in #3 you wrote

    I checked and the linkit filter runs before the spamspan filter.

    but that really doesn't make sense. If it was the case and the token is correct, it would have been replaced. Then this can not happen.

    You should check this, as we'll revert this commit in 🐛 SpamSpan breaks markup Fixed and you may run into these issues again. Feel free to reopen a followup and link this issue please, if that should happen. But first please check all your filters order and ensure SpamSpan runs last.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024