Problem/Motivation

This is a followup to #3511457: Don't replace term if there is already a link on this term in the page.
If a word is found in an element of the page that has the glossify-exclude class, it will be considered as already glossified and will not be replaced elsewhere in the page.

This is confusing for our users. They don't understand why we don't glossify a term because it is in a part of the page that has nothing to do with glossify.

Steps to reproduce

Proposed resolution

Don't add the term to $replaced if it was found in an element with the glossify-exclude class.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork glossify-3534898

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

prudloff created an issue. See original summary.

prudloff’s picture

Status: Active » Needs review

grevil made their first commit to this issue’s fork.

grevil’s picture

Version: 3.1.1 » 3.x-dev
Status: Needs review » Reviewed & tested by the community

Totally makes sense! Thanks for the fix!

I added a test to reproduce this issue. Here is the test output without the changes by @prudloff in place:

There was 1 failure:

1) Drupal\Tests\glossify\Unit\GlossifyBaseTest::testParseTooltipMatch with data set "set32" ('<div class="glossify-exclude"...ified.', [stdClass Object (...)], true, true, 'tooltips', false, '', '', '<div class="glossify-exclude"...ified.')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'...><a>RT</a></div> Here is <span title="Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularized in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.">RT</span> that should be glossified.'
+'...><a>RT</a></div> Here is RT that should be glossified.'

/var/www/html/web/modules/custom/glossify/tests/src/Unit/GlossifyBaseTest.php:35

  • grevil committed 25a42481 on 3.x authored by prudloff
    [#3534898] fix: Don't consider a term replaced if it is in an element...
grevil’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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