New unexpected classes now added to email

Thanks for reporting this. That's definitely wrong.

The expected result should be
<a class="btn btn-default spamspan">...</a>

We please need a test first to reproduce the issue and fix it forever.

Is there something special about the email address? Multiple dots or an "at" in the word? (that are other existing issues).

It is related to this issue:

3.1.4 released containing the fix for this issue.

Thanks @joevagyok I thought that was already merged and tagged! :)

3.1.4 adds a span with class="t" that I don't want.

<span class="spamspan"><span class="u">firstname.lastname</span> [at] <span class="d">mydomain.com</span><span class="t"> (firstnamedotlastnameatmydomaindotcom)</span></span>

How do I get rid of it? Temporary workaround:

.spamspan > .t { display: none; }

@iamfredrik looks like the JS is not running in your project? The t class is only temporary, being removed by the spamspan JS typically. Any console error?

@Anybody I have a decoupled front-end so there is no additional JS running that I haven't added myself. The t class was not there before updating to 3.1.4, so why has it been added? Can it be made optional?

@iamfredrik in that case, don't use spamspam or otherwise adapt the spamspan JS.

SpamSpan works in combination of a PHP Filter for obfuscation and de-obfuscation via JS. Your case is out of scope.

If you need something like that, please open a feature request and suggest changes there, which make it compatible. Thanks!

@Anybody, yes of course I have to adapt the JS to suit my scope. Just wondering why it unexpectedly got added after updating to 3.1.4. According to the documentation the t class is an optional anchor text class, so it would make sense to make it optional. Thanks!

Thanks @iamfredrik I'll reopen this for an additional check and test. The class should indeed only be present, if the anchor text is present, I think. But who ever fixes this, please prove me wrong.

Still, this is an edge-case, so if you're able to add a MR for a fix, that will speed things up a lot.

My issue was not entirely related to 📌 JavaScript not properly clearing the data attributes, when multiple classes are set Fixed . The update did indeed change the data-spamspan-btn-default to btn-default. But a bigger issue is how the btn btn-default classes got in there to begin with. They are not in my twig template which looks like <div class="icon-card-info-item">{{ content.field_contact_email.0 }}</div>. Searching for those classes among all of my code doesn't give explanation either. But here are the testing steps I've taken:

Roll back to 3.0.0-beta2 and those classes are not there.
Update to 3.1.5 and they are there.

With 3.1.5, modify the spamspan.js in my local dev by commenting out the entire script. There are no btn classes. Uncomment out the script, the classes return. With the spamspanjs commented out, the source shows this:

<div class="cta-links">
<div class="cta-link-1"><span class="spamspan" data-spamspan-class="btn btn-default" data-spamspan-target="_blank"><span class="u">osc</span> [at] <span class="d">mydomain.com</span><span class="t"> (<span class="text">Email Us</span>)</span></span></div>

<div class="cta-link-2"><a class="btn btn-default" href="tel:+15555555555" target="_blank"><span class="text">Call a Public Information Officer</span></a></div>

Now that section is at the bottom of the page, separate from the other email addresses. Could the script be picking up the btn btn-default classes that ARE indeed in that lower section and adding them to ALL instances of spamspan on the page?

I've tried doing an asset injector js script to remove the classes, but they remain. The asset injector script appears before spamspan.js and there doesn't appear to be a way for me to change the weight.

Here's the problematic part of the script that handles the extra attributes added in this commit (https://git.drupalcode.org/project/spamspan/-/commit/2146253dee3a0bfb0c0...):

// Check if the "span.spamspan" holds any extra attributes from the
// original <a> tag and put them back after removing 'data-spamspan-'
// string from the beginning.
let _attributes = '';
$("span.spamspan", context).each(function () {
  $.each(this.attributes, function () {
    if (this.specified && this.name.startsWith("data-spamspan-")) {
      _attributes += `${this.name.substring("data-spamspan-".length)}="${this.value}" `;
    }
  });
});
// Construct the <a> tag with the extra attributes, if there is any.
let _tag = "<a></a>";
if (_attributes) {
  _tag = `<a ${_attributes}></a>`;
}

The problem here is that the script is collecting attributes from all the "span.spamspan" elements within the given context, but it should only be considering the current element being processed in the iteration. So, attributes from later elements are being incorrectly applied to earlier ones.

I put an MR in place, but also included a patch file.

@shelane your findings are correct and the problem is well pointed out, thank you.
Tests are failing and I don't like the changes which are not directly related to the scope there, making the review process more complicated.
Would have been nice to see a clean change to the given problematic code snippet only, so we can see why tests are failing.

I will have a look at the problem.

I pushed a new branch with test changes to point out the problem first.

Okay, so I miss crutial information from #15 like:

1. What is the setup there? Is it a field, is it a WYSIWYG or a twig template?
2. What is the original input, and what is the expected output?
3. What would the phone number do with the email address links? It's never picked up by spamspan.

Even though your conclusion makes sense by looking at the JS and we iterate over span.spamspan inside the same iteration, I was not able to reproduce it via the test change.

I was able to reproduce the problem by applying an extra attribute to a later element which is not present on the previous elements. Fix is coming.

joevagyok → closed merge request !27

Looks like I pushed too fast the fix and the build meant to fail got interrupted by the fix build making it green. I will re-push the fail build without the fix.

Thanks again for finding the source of the issue @shelane!
MR clean and ready for review.

The last submitted patch, 18: 3376221-attributes.patch, failed testing. View results →

Uploading patch as well to prevent issue bot complains because of previously uploaded patch failing.

Apologies for the out of scope changes. The updated patch you provided is working to solve the issue. All email links are displaying properly.

Agree, RTBC +1!

Thanks all!

Released 3.1.6, with your changes.

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