Problem/Motivation

The suggestion label and descriptions are HTML-escaped in the matcher plugins. This becomes an issue when the values are then used in the user interface other than the LinkIt suggestion list. For example in #2712951: Linkit for Link field the label is injected into the Title for a Link field.

As a result, the URL Title displays escaped html entities:

Steps to reproduce

  1. Install Drupal Standard and enable LinkIt
  2. Create a basic page with title "Liberté, Égalité & Fraternité"
  3. Notice the & is escaped in the autocomplete suggestion at /linkit/autocomplete/default?q=Libert

Proposed resolution

Remove HTML escaping from the matcher plugins. Implement HTML escaping when displaying the suggestions instead.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

Data model changes

Matcher suggestions in the autocomplete json are no longer html escaped.

Original report by idflood

When inserting link in wysiwyg I had an issue with pages having a "&" in title.

To reproduce you can create a page with the title "Bugs & issues" for example, and then on another page you can try to link to it from the wysiwyg:
- When you click on the suggestion the textfield will get "Bugs & issues"
- When you submit and inspect the source of the wysiwyg you will notice that the only has the href attributes (and the href is equal to the title with the &).

The following patch only decode html entities for the entity label and fixes the issue locally.

CommentFileSizeAuthor
#30 afterpatch-linkit.png312.23 KBnivethasubramaniyan
#30 b4patch-linkit.png274.58 KBnivethasubramaniyan
#28 linkit-issue-linking-to-ampersands-2981543-28.patch2.21 KBidflood
#24 linkit-issue-linking-to-ampersands-2981543-24.patch2.16 KBericgsmith
#24 linkit-issue-linking-to-ampersands-2981543-24-test-only.patch1.61 KBericgsmith
#21 2981543-21.patch5.19 KBidebr
#21 2981543-21-test-only.patch1.26 KBidebr
#21 interdiff-18-21.txt955 bytesidebr
#20 2981543-19.patch4.41 KBanneke_vde
#18 2981543-18.patch4.65 KBidebr
#18 2981543-18-test-only.patch1.26 KBidebr
#18 2981543.gif98.31 KBidebr
#12 linkit-quotes-fixed.gif51.52 KBzero2one
#11 linkit-quotes.gif57.18 KBzero2one
#4 links_with_ampersand-2981543-4.patch488 bytesidflood
#2 links_with_ampersand-2981543-1.patch442 bytesidflood

Issue fork linkit-2981543

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:

About issue forks

Comments

idflood created an issue. See original summary.

idflood’s picture

idflood commented
StatusFileSize
new links_with_ampersand-2981543-1.patch442 bytes
idflood’s picture

idflood commented
Status: Active » Needs review
idflood’s picture

idflood commented
StatusFileSize
new links_with_ampersand-2981543-4.patch488 bytes

The following patch add a `htmlspecialchars_decode` since there was too an issue with single quote in titles (ex: That's it)

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Needs review » Needs work

- When you click on the suggestion the textfield will get "Bugs & issues"

What version of Linkit are you using?
This is not a default behavior, that's my I'm asking.

I tested with a node named "Tips & Tricks" and it works. The same with "Tips & 'tricks'".

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Needs work » Closed (cannot reproduce)
dddbbb’s picture

dddbbb commented
Title: Issue when linking to content with "&" in title » Issue when linking to content with ampersands or single quotes in the title
Status: Closed (cannot reproduce) » Reviewed & tested by the community

I've also run into this issue but with single quotes as mentioned in #4. The patch in #4 fixes the issue for me. I'm running the current latest dev release of linkit (#1084887).

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Reviewed & tested by the community » Needs work

Please add a test for this.

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Needs work » Closed (cannot reproduce)

This does not make any sense to me.

When you select a suggestion in the list, the only thing that will be populated is the "URL field". That field will be populated with the canonical url of the node (node/123).

I do not think you are using Linkit 8.x-5.x.

dddbbb’s picture

dddbbb commented

Confirmed. I was using an old dev version (dev-5.x#1084887). Have now updated to 5.0.0-beta9, retested and it seems I no longer need this patch.

zero2one’s picture

zero2one commented
Issue summary: View changes
Status: Closed (cannot reproduce) » Reviewed & tested by the community
StatusFileSize
new linkit-quotes.gif57.18 KB

I bumped into the same issue with the latest version of this module (8.x-5.0-beta9).

The patch in comment #4 fixes the issue.

In my case the node I was linking to has "Adoration de l'agneau mystique" as node title.

Without patch With patch
zero2one’s picture

zero2one commented
StatusFileSize
new linkit-quotes-fixed.gif51.52 KB
anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Reviewed & tested by the community » Needs work

This is strange as with the default behavior is that the node/nid should be populated when selecting a link from the suggestion list.

Is it a node you are trying to link to?

zero2one’s picture

zero2one commented

Yes it is a node.

The problem is that, without the patch, the ′ (not ' ) in the node title is HTML-encoded what results in broken functionality.

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented

@zero2one: Sorry but I still don't get it.

From your gifs, I can tell that you are using the wrong version. Are you 100% sure you are using the latest version of 8.x-5.x-dev?

rade’s picture

rade commented
Status: Needs work » Reviewed & tested by the community

I was also dealing with this issue. Patch from #4 fixes it.

anon’s picture

anon
Primary language Swedish
Location Gothenburg
commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Still, why does this becomes an issue? I cant reproduce it at all.
See #9

idebr’s picture

idebr commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new 2981543.gif98.31 KB
new 2981543-18-test-only.patch1.26 KB
new 2981543-18.patch4.65 KB

The suggestion label and descriptions are HTML-escaped in the matcher plugins. This becomes an issue when the values are then used in the user interface other than the LinkIt suggestion list. For example in #2712951: Linkit for Link field the label is injected into the Title for a Link field.

As a result, the URL Title displays escaped html entities:

Attached patch removes HTML escaping from the matcher plugins and implements HTML escaping when displaying the suggestions instead.

The last submitted patch, 18: 2981543-18-test-only.patch, failed testing. View results

anneke_vde’s picture

anneke_vde commented
StatusFileSize
new 2981543-19.patch4.41 KB

Attached patch fixed the error that I got:
Error: Class 'Drupal\linkit\Plugin\Linkit\Matcher\Html' not found in Drupal\linkit\Plugin\Linkit\Matcher\EntityMatcher->buildGroup() (regel 480 van /home/anneke/git/nji/htdocs/modules/contrib/linkit/src/Plugin/Linkit/Matcher/EntityMatcher.php)

idebr’s picture

idebr commented
StatusFileSize
new interdiff-18-21.txt955 bytes
new 2981543-21-test-only.patch1.26 KB
new 2981543-21.patch5.19 KB

#20 fixes the error, but uses an incorrect approach. Attached patch is based from #18 and updated with the latest changes from HEAD.

The last submitted patch, 21: 2981543-21-test-only.patch, failed testing. View results

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

ericgsmith’s picture

ericgsmith commented
StatusFileSize
new linkit-issue-linking-to-ampersands-2981543-24-test-only.patch1.61 KB
new linkit-issue-linking-to-ampersands-2981543-24.patch2.16 KB

I would like to propose an alternative suggestion based on the work in #3355004 which I will close as a duplicate issue.

Rather than removing escaping from the matcher plugin, we can handle the HTML clientside when inserting the value into the title field. This keeps the output from module correctly escaped. I believe it should be the responsibility of the thing using this data to correctly handle the escaped HTML.

No interdiff attached as this is an alternative approach.

ericgsmith’s picture

ericgsmith commented
Version: 8.x-5.x-dev » 6.0.x-dev

The last submitted patch, 24: linkit-issue-linking-to-ampersands-2981543-24-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

idebr’s picture

idebr commented

I believe it should be the responsibility of the thing using this data to correctly handle the escaped HTML.

Typically the output layer does the escaping to prevent duplicate escaping. For reference see #2297711: Fix HTML escaping due to Twig autoescape

idflood’s picture

idflood commented
StatusFileSize
new linkit-issue-linking-to-ampersands-2981543-28.patch2.21 KB

I wasn't able to apply patch in #24 to the current 6.0.0 so here is a reroll.

ericgsmith’s picture

ericgsmith commented

#27 - apologies I missed your reply.

RE:

Typically the output layer does the escaping to prevent duplicate escaping. For reference see #2297711: Fix HTML escaping due to Twig autoescape

I agree with this - Drupal is still an output layer via the endpoint - and my suggestion is to keep the Drupal application escaping at the output layer.

I believe this is similar to how core handles autocompletes - e.g for a standard entity autocomplete the html is escaped by Drupal so that the JS just renders what it receives https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/autocomp...

The difference here is after rendering we are wanting to take something that is HTML back into a plain text context - which I believe we can do by getting the text content of the html element instead of removing any escaping from the backend.

nivethasubramaniyan’s picture

nivethasubramaniyan commented
StatusFileSize
new b4patch-linkit.png274.58 KB
new afterpatch-linkit.png312.23 KB

I applied the patch in drupal 10.1.0 instance with linkit 6.1.0.
After applying the patch, it seems to be working fine . I have attached screenshots for the reference.

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented
Status: Needs review » Reviewed & tested by the community

The difference here is after rendering we are wanting to take something that is HTML back into a plain text context - which I believe we can do by getting the text content of the html element instead of removing any escaping from the backend.

This rationale makes sense to me, and the resolution is significantly more comprehensible than the approach of removing escaping from the matcher.

I'll proceed to merge this into both the 6.0.x branch (compatible with Drupal 9.x through 10.0.x) and the 6.1.x (compatible with Drupal 10.1.x+).

  • mark_fullmer committed 362de9af on 6.0.x 
    Issue #2981543 by idebr, idflood, ericgsmith, zero2one, mark_fullmer:...

  • mark_fullmer committed 2e81e22f on 6.1.x 
    Issue #2981543 by idebr, idflood, ericgsmith, zero2one, mark_fullmer:...
mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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