Skip to content

Commit 0b60b68

Browse files
FAMarfuatyFAMarfuaty
authored
Merge pull request #19688 from Yoast/19477-fix-highlighting-issues-in-classic-editor-and-shopify-3
[IM] Highlighting feature: Enable highlighting text containing html tags in Classic editor and make sure yoastmark doesn't corrupt anchor tags in Shopify
2 parents 8d2e0c3 + 916a709 commit 0b60b68

22 files changed

Lines changed: 367 additions & 36 deletions

File tree

‎packages/js/src/decorator/gutenberg.js‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import { create } from "@wordpress/rich-text";
99
import { select, dispatch } from "@wordpress/data";
1010
import getFieldsToMarkHelper from "./helpers/getFieldsToMarkHelper";
1111

12-
1312
const ANNOTATION_SOURCE = "yoast";
1413

1514
export const START_MARK = "<yoastmark class='yoast-text-mark'>";
15+
const START_MARK_DOUBLE_QUOTED = "<yoastmark class=\"yoast-text-mark\">";
1616
export const END_MARK = "</yoastmark>";
1717

1818
let annotationQueue = [];
@@ -114,6 +114,18 @@ export function isAnnotationAvailable() {
114114
*/
115115
export function getYoastmarkOffsets( marked ) {
116116
let startMarkIndex = marked.indexOf( START_MARK );
117+
118+
// Checks if the start mark is single quoted.
119+
// Note: if doesNotContainDoubleQuotedMark is true, this does necessary mean that the start mark is single quoted.
120+
// It could also be that the start mark doesn't occur at all in startMarkIndex.
121+
// In that case, startMarkIndex will be -1 during later tests.
122+
const doesNotContainDoubleQuotedMark = startMarkIndex >= 0;
123+
124+
// If the start mark is not found, try the double quoted version.
125+
if ( ! doesNotContainDoubleQuotedMark ) {
126+
startMarkIndex = marked.indexOf( START_MARK_DOUBLE_QUOTED );
127+
}
128+
117129
let endMarkIndex = null;
118130

119131
const offsets = [];
@@ -124,7 +136,8 @@ export function getYoastmarkOffsets( marked ) {
124136
* without the tags.
125137
*/
126138
while ( startMarkIndex >= 0 ) {
127-
marked = marked.replace( START_MARK, "" );
139+
marked = doesNotContainDoubleQuotedMark ? marked.replace( START_MARK, "" ) : marked.replace( START_MARK_DOUBLE_QUOTED, "" );
140+
128141
endMarkIndex = marked.indexOf( END_MARK );
129142

130143
if ( endMarkIndex < startMarkIndex ) {
@@ -137,7 +150,8 @@ export function getYoastmarkOffsets( marked ) {
137150
endOffset: endMarkIndex,
138151
} );
139152

140-
startMarkIndex = marked.indexOf( START_MARK );
153+
startMarkIndex = doesNotContainDoubleQuotedMark ? marked.indexOf( START_MARK ) : marked.indexOf( START_MARK_DOUBLE_QUOTED );
154+
141155
endMarkIndex = null;
142156
}
143157

‎packages/js/src/decorator/tinyMCE.js‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { markers } from "yoastseo";
1+
import { markers, languageProcessing } from "yoastseo";
22
import { forEach } from "lodash-es";
3-
import { languageProcessing } from "yoastseo";
43

54
var MARK_TAG = "yoastmark";
65

@@ -36,11 +35,30 @@ function markTinyMCE( editor, paper, marks ) {
3635
let html = editor.getContent();
3736
html = markers.removeMarks( html );
3837

38+
/*
39+
* 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.
40+
* For example, [ "heading" ] means that we want to apply the markings in subheadings only, and not the other parts.
41+
* `selectedHTML` is an array of the HTML parts that we want to apply the marking to.
42+
*/
3943
const { fieldsToMark, selectedHTML } = languageProcessing.getFieldsToMark( marks, html );
4044

4145
// Generate marked HTML.
4246
forEach( marks, function( mark ) {
47+
/*
48+
* Classic editor uses double quotes for HTML attribute values. However, Block editor uses single quotes for HTML tag attributes,
49+
* and that's why in `yoastseo`, we use single quotes for the attribute values when we create the marked object. As a result,
50+
* the replacement did not work, as the marks passed by `yoastseo` did not match anything in the original text.
51+
* This step is replacing the single quotes in the marked object output by `yoastseo` with double quotes.
52+
* 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.
53+
*/
54+
if ( editor.id !== "acf_content" ) {
55+
mark._properties.marked = languageProcessing.normalizeHTML( mark._properties.marked );
56+
mark._properties.original = languageProcessing.normalizeHTML( mark._properties.original );
57+
}
58+
59+
// Check if we want to mark only specific part of the HTML.
4360
if ( fieldsToMark.length > 0 ) {
61+
// Apply the marking to the selected HTML parts.
4462
selectedHTML.forEach( element => {
4563
const markedElement = mark.applyWithReplace( element );
4664
html = html.replace( element, markedElement );

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ const expectedResults = {
119119
textTransitionWords: {
120120
isApplicable: true,
121121
score: 6,
122-
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 23.5% of the sentences contain transition words, " +
122+
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 23.1% of the sentences contain transition words, " +
123123
"which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
124124
},
125125
passiveVoice: {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,13 @@ const expectedResults = {
127127
textTransitionWords: {
128128
isApplicable: true,
129129
score: 6,
130-
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 27.1% of the sentences contain " +
130+
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 25.5% of the sentences contain " +
131131
"transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
132132
},
133133
passiveVoice: {
134134
isApplicable: true,
135135
score: 3,
136-
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 16.7% of the sentences contain passive voice, " +
136+
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 15.7% of the sentences contain passive voice, " +
137137
"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
138138
"Try to use their active counterparts</a>.",
139139
},

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,13 @@ const expectedResults = {
126126
textTransitionWords: {
127127
isApplicable: true,
128128
score: 3,
129-
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 19.7% of the sentences contain" +
129+
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 19.4% of the sentences contain" +
130130
" transition words, which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
131131
},
132132
passiveVoice: {
133133
isApplicable: true,
134134
score: 3,
135-
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 25.9% of the sentences contain passive voice, " +
135+
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 25.5% of the sentences contain passive voice, " +
136136
"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
137137
"Try to use their active counterparts</a>.",
138138
},

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ const expectedResults = {
126126
textTransitionWords: {
127127
isApplicable: true,
128128
score: 3,
129-
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 5.6% of the sentences contain transition words, " +
129+
resultText: "<a href='https://yoa.st/34z' target='_blank'>Transition words</a>: Only 5.5% of the sentences contain transition words, " +
130130
"which is not enough. <a href='https://yoa.st/35a' target='_blank'>Use more of them</a>.",
131131
},
132132
passiveVoice: {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ const expectedResults = {
135135
passiveVoice: {
136136
isApplicable: true,
137137
score: 6,
138-
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 11.8% of the sentences contain passive voice, " +
138+
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 11.5% of the sentences contain passive voice, " +
139139
"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
140140
"Try to use their active counterparts</a>.",
141141
},

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ const expectedResults = {
112112
textTransitionWords: {
113113
isApplicable: true,
114114
score: 3,
115-
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>.",
115+
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>.",
116116
},
117117
passiveVoice: {
118118
isApplicable: true,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ const expectedResults = {
119119
passiveVoice: {
120120
isApplicable: true,
121121
score: 3,
122-
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>.",
122+
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>.",
123123
},
124124
textPresence: {
125125
isApplicable: true,

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,8 @@ const expectedResults = {
126126
},
127127
passiveVoice: {
128128
isApplicable: true,
129-
score: 6,
130-
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: 10.6% of the sentences contain passive voice, " +
131-
"which is more than the recommended maximum of 10%. <a href='https://yoa.st/34u' target='_blank'>" +
132-
"Try to use their active counterparts</a>.",
129+
score: 9,
130+
resultText: "<a href='https://yoa.st/34t' target='_blank'>Passive voice</a>: You're using enough active voice. That's great!",
133131
},
134132
textPresence: {
135133
isApplicable: true,

0 commit comments

Comments
 (0)
⚡