[IM] Highlighting feature: Enable highlighting text containing html tags in Classic editor and make sure yoastmark doesn't corrupt anchor tags in Shopify

packages/js/src/decorator/gutenberg.js

@@ -9,10 +9,10 @@ import { create } from "@wordpress/rich-text";
 import { select, dispatch } from "@wordpress/data";
 import getFieldsToMarkHelper from "./helpers/getFieldsToMarkHelper";
 
-
 const ANNOTATION_SOURCE = "yoast";
 
 export const START_MARK = "<yoastmark class='yoast-text-mark'>";
+const START_MARK_DOUBLE_QUOTED = "<yoastmark class=\"yoast-text-mark\">";
 export const END_MARK =   "</yoastmark>";
 
 let annotationQueue = [];
@@ -114,6 +114,18 @@ export function isAnnotationAvailable() {
  */
 export function getYoastmarkOffsets( marked ) {
 	let startMarkIndex = marked.indexOf( START_MARK );
+
+	// Checks if the start mark is single quoted.
+	// Note: if doesNotContainDoubleQuotedMark is true, this does necessary mean that the start mark is single quoted.
+	// It could also be that the start mark doesn't occur at all in startMarkIndex.
+	// In that case, startMarkIndex will be -1 during later tests.
+	const doesNotContainDoubleQuotedMark = startMarkIndex >= 0;
+
+	// If the start mark is not found, try the double quoted version.
+	if ( ! doesNotContainDoubleQuotedMark ) {
+		startMarkIndex = marked.indexOf( START_MARK_DOUBLE_QUOTED );
+	}
+
 	let endMarkIndex = null;
 
 	const offsets = [];
@@ -124,7 +136,8 @@ export function getYoastmarkOffsets( marked ) {
 	 * without the tags.
 	 */
 	while ( startMarkIndex >= 0 ) {
-		marked = marked.replace( START_MARK, "" );
+		marked = doesNotContainDoubleQuotedMark ? marked.replace( START_MARK, "" ) : marked.replace( START_MARK_DOUBLE_QUOTED, "" );
+
 		endMarkIndex = marked.indexOf( END_MARK );
 
 		if ( endMarkIndex < startMarkIndex ) {
@@ -137,7 +150,8 @@ export function getYoastmarkOffsets( marked ) {
 			endOffset: endMarkIndex,
 		} );
 
-		startMarkIndex = marked.indexOf( START_MARK );
+		startMarkIndex = doesNotContainDoubleQuotedMark ? marked.indexOf( START_MARK ) : marked.indexOf( START_MARK_DOUBLE_QUOTED );
+
 		endMarkIndex = null;
 	}
 

packages/js/src/decorator/tinyMCE.js

@@ -1,6 +1,5 @@
-import { markers } from "yoastseo";
+import { markers, languageProcessing } from "yoastseo";
 import { forEach } from "lodash-es";
-import { languageProcessing } from "yoastseo";
 
 var MARK_TAG = "yoastmark";
 
@@ -36,11 +35,30 @@ function markTinyMCE( editor, paper, marks ) {
 	let html = editor.getContent();
 	html = markers.removeMarks( html );
 
+	/*
+	 * Get the information whether we want to mark a specific part of the HTML. If we do, `fieldsToMark` should return an array with that information.
+	 * For example, [ "heading" ] means that we want to apply the markings in subheadings only, and not the other parts.
+	 * `selectedHTML` is an array of the HTML parts that we want to apply the marking to.
+	 */
 	const { fieldsToMark, selectedHTML } = languageProcessing.getFieldsToMark( marks, html );
 
 	// Generate marked HTML.
 	forEach( marks, function( mark ) {
+		/*
+		 * Classic editor uses double quotes for HTML attribute values. However, Block editor uses single quotes for HTML tag attributes,
+		 * and that's why in `yoastseo`, we use single quotes for the attribute values when we create the marked object. As a result,
+		 * the replacement did not work, as the marks passed by `yoastseo` did not match anything in the original text.
+		 * This step is replacing the single quotes in the marked object output by `yoastseo` with 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.
+		 */
+		if ( editor.id !== "acf_content" ) {
+			mark._properties.marked = languageProcessing.normalizeHTML( mark._properties.marked );
+			mark._properties.original = languageProcessing.normalizeHTML( mark._properties.original );
+		}
+
+		// Check if we want to mark only specific part of the HTML.
 		if ( fieldsToMark.length > 0 ) {
+			// Apply the marking to the selected HTML parts.
 			selectedHTML.forEach( element => {
 				const markedElement = mark.applyWithReplace( element );
 				html = html.replace( element, markedElement );

packages/yoastseo/spec/fullTextTests/testTexts/ar/arabicPaper.js

@@ -119,7 +119,7 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 6,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 23.5% of the sentences contain transition words, " +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 23.1% of the sentences contain transition words, " +
 			"which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {

packages/yoastseo/spec/fullTextTests/testTexts/de/germanPaper.js

@@ -127,13 +127,13 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 6,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 27.1% of the sentences contain " +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 25.5% of the sentences contain " +
 			"transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 16.7% of the sentences contain passive voice, " +
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 15.7% of the sentences contain passive voice, " +
 			"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
 			"Try to use their active counterparts</a>.",
 	},

packages/yoastseo/spec/fullTextTests/testTexts/el/greekPaper.js

@@ -126,13 +126,13 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 19.7% of the sentences contain" +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 19.4% of the sentences contain" +
 			" transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 25.9% of the sentences contain passive voice, " +
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 25.5% of the sentences contain passive voice, " +
 			"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
 			"Try to use their active counterparts</a>.",
 	},

packages/yoastseo/spec/fullTextTests/testTexts/en/englishPaperForPerformanceTest.js

@@ -126,7 +126,7 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 5.6% of the sentences contain transition words, " +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 5.5% of the sentences contain transition words, " +
 			"which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {

packages/yoastseo/spec/fullTextTests/testTexts/fa/farsiPaper.js

@@ -135,7 +135,7 @@ const expectedResults = {
 	passiveVoice: {
 		isApplicable: true,
 		score: 6,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 11.8% of the sentences contain passive voice, " +
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 11.5% of the sentences contain passive voice, " +
 			"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
 			"Try to use their active counterparts</a>.",
 	},

packages/yoastseo/spec/fullTextTests/testTexts/hu/hungarianPaper.js

@@ -112,7 +112,7 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 13.1% of the sentences contain transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 12.5% of the sentences contain transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {
 		isApplicable: true,

packages/yoastseo/spec/fullTextTests/testTexts/id/indonesianPaper.js

@@ -119,7 +119,7 @@ const expectedResults = {
 	passiveVoice: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 25.3% of the sentences contain passive voice, which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>Try to use their active counterparts</a>.",
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 24.7% of the sentences contain passive voice, which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>Try to use their active counterparts</a>.",
 	},
 	textPresence: {
 		isApplicable: true,

packages/yoastseo/spec/fullTextTests/testTexts/nb/norwegianPaper.js

@@ -126,10 +126,8 @@ const expectedResults = {
 	},
 	passiveVoice: {
 		isApplicable: true,
-		score: 6,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 10.6% of the sentences contain passive voice, " +
-			"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
-			"Try to use their active counterparts</a>.",
+		score: 9,
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: You're using enough active voice. That's great!",
 	},
 	textPresence: {
 		isApplicable: true,

packages/yoastseo/spec/fullTextTests/testTexts/pt/portuguesePaper.js

@@ -127,13 +127,13 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 7.8% of the sentences contain transition words," +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 7.6% of the sentences contain transition words," +
 			" which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {
 		isApplicable: true,
 		score: 6,
-		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 10.6% of the sentences contain passive voice, " +
+		resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 10.4% of the sentences contain passive voice, " +
 			"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
 			"Try to use their active counterparts</a>.",
 	},

packages/yoastseo/spec/fullTextTests/testTexts/sk/slovakPaper.js

@@ -121,7 +121,7 @@ const expectedResults = {
 	textTransitionWords: {
 		isApplicable: true,
 		score: 3,
-		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 12.9% of the sentences contain transition words," +
+		resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 12.8% of the sentences contain transition words," +
 			" which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
 	},
 	passiveVoice: {

packages/yoastseo/spec/fullTextTests/testTexts/tr/turkishPaper.js

@@ -90,8 +90,10 @@ const expectedResults = {
 		isApplicable: false,
 	},
 	keyphraseDistribution: {
-		// The text doesnt contain more than 15 sentences.
-		isApplicable: false,
+		isApplicable: true,
+		score: 6,
+		resultText: "<a href='https://yoa.st/33q' target='_blank'>Keyphrase distribution</a>: Uneven. Some parts of your text do not " +
+			"contain the keyphrase or its synonyms. <a href='https://yoa.st/33u' target='_blank'>Distribute them more evenly</a>.",
 	},
 	subheadingsTooLong: {
 		isApplicable: true,

packages/yoastseo/spec/languageProcessing/helpers/html/normalizeHTMLSpec.js

@@ -0,0 +1,38 @@
+import normalizeHTML from "../../../../src/languageProcessing/helpers/html/normalizeHTML";
+
+describe( "normalizeHTML", function() {
+	it( "should return the same string when no single quotes are present", function() {
+		expect( normalizeHTML( "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( normalizeHTML( "<yoastmark class=\"yoast-text-mark\">This is a test</yoastmark>" ) )
+			.toEqual( "<yoastmark class=\"yoast-text-mark\">This is a test</yoastmark>" );
+	} );
+
+	it( "should return the same string when a string contains non breaking spaces.", function() {
+		expect( normalizeHTML( "<yoastmark class=\"yoast-text-mark\">This\u{00a0}is a\u{00a0}test</yoastmark>" ) )
+			.toEqual( "<yoastmark class=\"yoast-text-mark\">This\u{00a0}is a\u{00a0}test</yoastmark>" );
+	} );
+
+	it( "should not replace single quotes (or apostrophes) outside HTML tags", function() {
+		expect( normalizeHTML( "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( normalizeHTML( "<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( normalizeHTML( "<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( normalizeHTML( "<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>" );
+	} );
+} );

packages/yoastseo/spec/languageProcessing/helpers/sentence/SentenceTokenizerSpec.js

@@ -419,3 +419,34 @@ describe( "A test for tokenizing a (html) text into sentences", function() {
 	} );
 } );
 
+describe( "testing the isValidTagPair helper method", function() {
+	it( "returns true if the tags are of the same type and the correct type", function() {
+		[ "p", "div", "h1", "h2", "h3", "h4", "h5", "h6", "span" ].forEach( function( tagType ) {
+			const mockOpenTag = { src: `<${tagType}>` };
+			const mockCloseTag = { src: `</${tagType}>` };
+			expect( mockTokenizer.isValidTagPair( mockOpenTag, mockCloseTag ) ).toBe( true );
+		} );
+	} );
+	it( "returns true if the tags are of the same type and the correct type and the have attributes", function() {
+		[ "p", "div", "h1", "h2", "h3", "h4", "h5", "h6", "span" ].forEach( function( tagType ) {
+			const mockOpenTag = { src: `<${tagType} class="onzin" messy="1">` };
+			const mockCloseTag = { src: `</${tagType}>` };
+			expect( mockTokenizer.isValidTagPair( mockOpenTag, mockCloseTag ) ).toBe( true );
+		} );
+	} );
+	it( "returns false if the tags are of the same type but not of the correct type", function() {
+		const mockOpenTag = { src: "<i>" };
+		const mockCloseTag = { src: "</i>" };
+		expect( mockTokenizer.isValidTagPair( mockOpenTag, mockCloseTag ) ).toBe( false );
+	} );
+	it( "returns false if the tags are of te correct type but not of the same type", function() {
+		const mockOpenTag = { src: "<div>" };
+		const mockCloseTag = { src: "</span>" };
+		expect( mockTokenizer.isValidTagPair( mockOpenTag, mockCloseTag ) ).toBe( false );
+	} );
+	it( "returns false if the tags are of te wrong type and not of the same type", function() {
+		const mockOpenTag = { src: "<i>" };
+		const mockCloseTag = { src: "</b>" };
+		expect( mockTokenizer.isValidTagPair( mockOpenTag, mockCloseTag ) ).toBe( false );
+	} );
+} );