PC-965 Fixes highlighting when inline HTML tags are present in Classic editor and in Shopify #19162
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import replaceSingleQuotesInTags from "../../../../src/languageProcessing/helpers/html/replaceQuotes"; | ||
|
|
||
| describe( "replace-quotes", function() { | ||
| describe( "replaceSingleQuotesInTags", function() { | ||
| it( "should return the same string when no single quotes are present", function() { | ||
| expect( replaceSingleQuotesInTags( "This is a test" ) ) | ||
| .toEqual( "This is a test" ); | ||
| } ); | ||
|
|
||
| it( "should return the same string when only double quotes in HTML attribute values are present", function() { | ||
| expect( replaceSingleQuotesInTags( "<yoastmark class=\"yoast-text-mark\">This is a test</yoastmark>" ) ) | ||
| .toEqual( "<yoastmark class=\"yoast-text-mark\">This is a test</yoastmark>" ); | ||
| } ); | ||
|
|
||
| it( "should not replace single quotes outside HTML tags", function() { | ||
| expect( replaceSingleQuotesInTags( "This is a test, let's go!" ) ) | ||
| .toEqual( "This is a test, let's go!" ); | ||
| } ); | ||
|
|
||
| it( "should replace the outer single quotes in HTML attribute values with double quotes", function() { | ||
| expect( replaceSingleQuotesInTags( "<span style='color: red'>This</span> is a test" ) ) | ||
| .toEqual( "<span style=\"color: red\">This</span> is a test" ); | ||
| } ); | ||
|
|
||
| it( "should not replace any inner single quotes in HTML attribute values", function() { | ||
| expect( replaceSingleQuotesInTags( "<span data-attr=\"let's go, time's up\">This</span> is a test" ) ) | ||
| .toEqual( "<span data-attr=\"let's go, time's up\">This</span> is a test" ); | ||
| } ); | ||
|
|
||
| it( "should replace the outer single quotes in multiple HTML attribute values with double quotes", function() { | ||
| expect( replaceSingleQuotesInTags( "<yoastmark class='yoast-text-mark' style='color: blue'>This is a test</yoastmark>" ) ) | ||
| .toEqual( "<yoastmark class=\"yoast-text-mark\" style=\"color: blue\">This is a test</yoastmark>" ); | ||
| } ); | ||
| } ); | ||
| } ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| /** | ||
| * Replaces single quotes around HTML attribute values with double quotes. | ||
| * Double quotes are the standard, but we convert these to single quotes when parsing the HTML in `yoastseo` package. | ||
| * Here, we change them back to double quotes so by parsing the HTML and then outputting it again. | ||
| * | ||
| * @param {string} str The input string. | ||
| * | ||
| * @returns {string} The string with single quotes around HTML attributes replaced with double quotes. | ||
| */ | ||
| export default function( str ) { | ||
| const element = document.createElement( "body" ); | ||
| element.innerHTML = str; | ||
| return element.innerHTML; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,33 @@ | ||
| import getAnchorsFromText from "../link/getAnchorsFromText"; | ||
| import matchWords from "../match/matchTextWithArray"; | ||
| import arrayToRegex from "../regex/createRegexFromArray"; | ||
| import addMark from "../../../markers/addMarkSingleWord"; | ||
| import Mark from "../../../values/Mark"; | ||
| import { escapeRegExp } from "lodash-es"; | ||
| import { stripFullTags } from "../sanitize/stripHTMLTags"; | ||
|
|
||
| /** | ||
| * Gets the anchors and mark the anchors' text if the topic is found in the anchors' text. | ||
| * | ||
| * @param {string} sentence The sentence to retrieve the anchors from. | ||
| * @param {RegExp} topicRegex The regex of the topic. | ||
| * | ||
| * @returns {Object} The anchors and the marked anchors. | ||
| */ | ||
| const getMarkedAnchors = function( sentence, topicRegex ) { | ||
| // Retrieve the anchors. | ||
| const anchors = getAnchorsFromText( sentence ); | ||
| // For every anchor, apply the markings only to the anchor tag. Replace the unmarked anchor in the sentence with the marked anchor. | ||
| const markedAnchors = anchors.map( anchor => { | ||
| // Get the anchor text. | ||
| const anchorText = stripFullTags( anchor ); | ||
| // Apply the marking to the anchor text. | ||
| const markedAnchorText = anchorText.replace( topicRegex, ( x ) => addMark( x ) ); | ||
| // Replace the original anchor text with the marked anchor text. | ||
| return anchor.replace( anchorText, markedAnchorText ); | ||
| } ); | ||
| return { anchors, markedAnchors }; | ||
| }; | ||
|
|
||
| /** | ||
| * Adds marks to a sentence and merges marks if those are only separated by a space | ||
|
|
@@ -19,10 +44,27 @@ export const collectMarkingsInSentence = function( sentence, topicFoundInSentenc | |
| topicFoundInSentence = topicFoundInSentence.map( word => escapeRegExp( word ) ); | ||
| // If a language has a custom helper to match words, we disable the word boundary when creating the regex. | ||
| const topicRegex = matchWordCustomHelper ? arrayToRegex( topicFoundInSentence, true ) : arrayToRegex( topicFoundInSentence ); | ||
|
|
||
| // Retrieve the anchors and mark the anchors' text if the topic is found in the anchors' text. | ||
| const { anchors, markedAnchors } = getMarkedAnchors( sentence, topicRegex ); | ||
|
|
||
| let markup = sentence.replace( topicRegex, function( x ) { | ||
| return addMark( x ); | ||
| } ); | ||
|
|
||
| /** | ||
| * In 'markup', we apply the markings also inside the anchor's attribute if there is a match, on top of | ||
| * marking the anchor's text. | ||
| * The step below is to replace the incorrectly marked anchors with the marked anchors that we want: | ||
| * where the markings are only applied in the anchor's text. | ||
| */ | ||
| if ( anchors.length > 0 ) { | ||
| const markupAnchors = getAnchorsFromText( markup ); | ||
| for ( let i = 0; i < markupAnchors.length; i++ ) { | ||
| markup = markup.replace( markupAnchors[ i ], markedAnchors[ i ] ); | ||
| } | ||
| } | ||
|
Comment on lines
+90
to
+101
Contributor
Author
There was a problem hiding this comment. The approach is as follows:
This approach works, but I have to admit that it's a little bit convoluted.
Contributor
Author
There was a problem hiding this comment. @agnieszkaszuba 's suggestion for improvement for this approach:
The advantage of this approach is that we don’t need to mark the text between alt tags twice, like it’s currently done in
Contributor
Author
There was a problem hiding this comment. We tried to implement the above suggestion. However, the third step "Replace the tags in the marked text with the original tags" proved to be tricky. And we couldn't formulate a regex that can detect anchor opening tag that includes the Hence, we decided to use the current approach. |
||
|
|
||
| return ( markup.replace( new RegExp( "</yoastmark> <yoastmark class='yoast-text-mark'>", "ig" ), " " ) ); | ||
| }; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classic editor uses double quotes for HTML attribute values. However, in
yoastseo, we use single quotes for the attribute values when we create the marked object. As the result, the replacement did not work, as the marks passed byyoastseodid not match anything in the original text.This step is replacing the single quotes in the marked object output by
yoastseowith double quotes. This way, we make sure that the replacement can find a match between the original text of the marked object and the text in the page.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
yoastseo, single quotes are used because Block editor uses single quotes for HTML tag attributes.