Prevent duplicate processing of the same text

Created on 15 December 2024, 6 months ago

Problem/Motivation

There doesn't seem to be anything to prevent duplicate processing of a glossary term. This creates weird markup as glossify passes over the term twice, and embeds another glossary

Steps to reproduce

This came up for us when we had an embedded entity using entity_embed that had a rich text field with a glossary term in it. Glossify parsed it when rendering that rich text and added the tooltip. Then, when the rich text field that embedded the entity was processed, it found the glossary term again and processed it again.

Proposed resolution

When finding text to check against glossary terms, the base text filter has this code:

$text_nodes = $xpath->query('//text()[not(ancestor::a) and not(ancestor::span[@class="glossify-exclude"])]');

How about we add the glossify-exclude class to the default template for the tooltip, and then change the selector slightly to this:

$text_nodes = $xpath->query('//text()[not(ancestor::a) and not(ancestor::span[contains(@class, "glossify-exclude")])]');

This change uses a "contains" instead of exact match for class name. This allows flexibility in themers that override the tooltip template to add additional classes to a wrapping span.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report

Status

Active

Version

3.0

Component

Code

Created by

🇺🇸United States bkosborne New Jersey, USA

Live updates comments and jobs are added and updated live.

Merge Requests

!50Prevent duplicate processing of the same text
Open
🇺🇸United States bkosborne
updated 12 days ago

Comments & Activities

Issue created by @bkosborne
Comment 4 months ago →
🇩🇪Germany Anybody Porta Westfalica
@bkosborne nice idea, I think you could prepare this as MR to see if it works as expected?
I don't have any other ideas yet.

Due to the complexity of this module I think most important is to have good tests for every possible edge case...
Comment 12 days ago →
🇺🇸United States bkosborne New Jersey, USA
Looks like the selector was already updated to use a "contains" query in the past few releases, so all that's needed here is to add the "glossify-exclude" class to the default template for tooltips. I'll create an MR for that and see if existing tests pass.
Merge request !50Ensure that rendered tooltips are not further processed if the are run through... → (Open) created by bkosborne
Pipeline finished with Success
12 days ago
Total: 140s
#501162

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024