Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#26307 closed defect (bug) (duplicate)

Link tool in basic editor should escape quotes

Reported by: krinklemail Owned by:
Milestone: Priority: normal
Severity: major Version: 3.7.1
Component: Editor Keywords: has-patch
Focuses: Cc:


When inserting links, I quite often use quotes inside the link tooltip (e.g. a tooltip like: Lookup "foo" on Wiktionary. Or Read more about "Foo" on Wikipedia.

However these always end up being inserted in the editor unescaped.

Example text
 <a href="https://en.wiktionary.org/wiki/whilst" title="Lookup "whilst" on Wiktionary" target="_blank">whilst</a>
referring to
 <a href="https://en.wikipedia.org/wiki/Elementary_(TV_series)" title="Read about "Elementary (TV series)" on Wikipedia" target="_blank">Elementary</a>
on Wikipedia.

The attribute values for "title" and "href" (like any attribute value, really) need escaping (especially tooltip, which is more likely contain a quote, though the URL should naturally have html special characters escaped as well).

Attachments (2)

26307.patch (484 bytes) - added by SergeyBiryukov 4 years ago.
26307.2.patch (484 bytes) - added by JanHenkG 4 years ago.

Download all attachments as: .zip

Change History (10)

#1 @krinklemail
4 years ago

  • Cc timotijhof@… added
  • Severity changed from normal to major

#2 @SergeyBiryukov
4 years ago

  • Keywords has-patch added

Reproduced in Text editor.

TinyMCE turns the quotes into &quot;. 26307.patch does the same for Text editor.

4 years ago

#3 @JanHenkG
4 years ago

Updated the patch of SergeyBiryukov because the affected lines changed in the trunk. The change in the patch is still the same, and I can confirm it fixes the bug.

#4 @JanHenkG
4 years ago

  • Cc j.h.gerritsen@… added

#5 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 3.9

Generally, a refresh is only needed when the patch no longer applies cleanly.

#6 @azaozz
4 years ago

The patch makes sense. Perhaps it should escape the other "htmlspecialchars" too, something like:

if ( attrs.title ) {
  var title = attrs.title.replace( /"/g, '&quot;' ).replace( /</g, '&lt;' ).replace( />/g, '&gt;' );
  html += ' title="' + title + '"';

#7 @JanHenkG
4 years ago

A good function to escape HTML in Javascript is the following function:

function escapeHtml(text) {
    return text
        .replace(/&/g, "&amp;")
        .replace(/</g, "&lt;")
        .replace(/>/g, "&gt;")
        .replace(/"/g, "&quot;")
        .replace(/'/g, "&#039;");

The easiest fix is to just inline this function and only apply it to the title attribute of the link being build. But I can imagine that the functionality for escaping HTML could be reused elsewhere in the future, so maybe there is a better location to put this function? Maybe as a function in the wp-includes/js/utils.js file? Or maybe in a new Javascript file, for example wp-includes/js/formatting.js ?

I am new to WordPress contributing, so if someone could tell me what would be the best approach in line with the WordPress coding standards, I am happy to implement it and provide a patch.

#8 @SergeyBiryukov
4 years ago

  • Milestone 3.9 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #25704.

Note: See TracTickets for help on using tickets.