diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/EditableChipList/index.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/EditableChipList/index.vue index aadc8ff03b..ea7af394ce 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/EditableChipList/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/EditableChipList/index.vue @@ -167,9 +167,9 @@ import ClickableRegion from '../ClickableRegion/index.vue'; import ValidationMessage from '../ValidationMessage/index.vue'; import { qtiEditorStrings } from '../../qtiEditorStrings'; - import { hasRichTextContent } from '../../utils/richText'; import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; + import { hasRichTextContent } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; // Holds a message line open without text const NO_BREAK_SPACE = '\u00a0'; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInlineChoiceInteraction.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInlineChoiceInteraction.js index 7dd01f1227..477e600e28 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInlineChoiceInteraction.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInlineChoiceInteraction.js @@ -1,8 +1,8 @@ import { computed, readonly } from 'vue'; import { inlineChoiceInteractionDescriptor } from '../interactions/inlineChoice/Descriptor'; import { getDropdowns } from '../interactions/inlineChoice/parse'; -import { hasRichTextContent } from '../utils/richText'; import { useInteraction } from './useInteraction'; +import { hasRichTextContent } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Composable for the inline choice interaction editor. diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/Editor.vue index 174e989a49..bae7fd89db 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/Editor.vue @@ -248,9 +248,12 @@ import ClickableRegion from '../../components/ClickableRegion/index.vue'; import ShuffledResponsePool from '../../components/ShuffledResponsePool/index.vue'; import EditableChipList from '../../components/EditableChipList/index.vue'; - import { hasRichTextContent } from '../../utils/richText'; + import { resolveAssociateChoices } from './parse'; import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; - import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + import { + hasRichTextContent, + richTextComparisonKey, + } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; /** Which card currently holds the open TipTap editor. */ @@ -531,27 +534,21 @@ props.mode === 'edit' ? correctPairsLabel$() : correctAnswersLabel$(), ); - // Blank pair members are left out: every blank choice keys the same, so - // keeping them would mark an unwritten option in the pool as correct. - const pairedTexts = computed( - () => - new Set( - flatten(state.value.pairs) - .filter(choice => hasRichTextContent(choice.content)) - .map(choice => richTextComparisonKey(choice.content)), - ), - ); - - // One option per association the learner can make: the emitted XML folds - // choices holding the same answer into a single option whose match-max is - // the number of occurrences walked here. - const poolChoices = computed(() => - [...flatten(state.value.pairs), ...state.value.distractors].map(choice => ({ - content: choice.content, - isCorrect: - props.showAnswers && pairedTexts.value.has(richTextComparisonKey(choice.content)), - })), - ); + // Kolibri's pool shows each choice of the interaction once. + const poolChoices = computed(() => { + const { pairs, pool } = resolveAssociateChoices(state.value.pairs, state.value.distractors); + // Blank pair members are left out: every blank choice keys the same, so + // keeping them would mark an unwritten option in the pool as correct. + const pairedKeys = new Set( + flatten(pairs) + .map(({ key }) => key) + .filter(Boolean), + ); + return pool.map(({ content, key }) => ({ + content, + isCorrect: props.showAnswers && pairedKeys.has(key), + })); + }); return { EditorImageProcessor, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js index fc5e8dfdb9..c85198d4c9 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/__tests__/Editor.spec.js @@ -11,6 +11,7 @@ import { mockInteractionBlockWithDecl as blockWithDecl, } from '../../../utils/testingFixtures'; import { QuestionType } from '../../../constants'; +import { parseXML } from '../../../serialization/xml'; import { qtiEditorStrings as tr } from '../../../qtiEditorStrings'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); @@ -60,6 +61,37 @@ const SHARED_CHOICE_DECL_XML = ` `; +const PAIRED_DISTRACTOR_XML = ` + Match each character to his adversary. + Antonio + Prospero + Antonio +`; + +const BLANK_CHOICES_XML = ` + Match each character to his adversary. + + Prospero + Capulet + +`; + +const THRICE_SHARED_CHOICE_XML = ` + Match each character to his adversary. + Antonio + Prospero + Capulet + Montague +`; + +const THRICE_SHARED_CHOICE_DECL_XML = ` + + choice_aaa11111 choice_bbb22222 + choice_aaa11111 choice_ccc33333 + choice_aaa11111 choice_ddd44444 + +`; + // The mock TipTapEditor renders a only for the card currently open. const openTextarea = () => screen.queryAllByRole('textbox').find(el => el.tagName === 'TEXTAREA'); @@ -376,13 +408,49 @@ describe('AssociateEditor', () => { } }); - it('repeats an option in the pool once per association it allows', () => { + it('lists an option shared by several associations once in the pool', () => { renderEditor({ ...viewProps, interaction: blockWithDecl(SHARED_CHOICE_XML, SHARED_CHOICE_DECL_XML), }); - expect(screen.getAllByText('Antonio')).toHaveLength(2); - expect(screen.getAllByText('Prospero')).toHaveLength(1); + const pool = screen.getByRole('list', { name: tr.$tr('responsePoolLabel') }); + expect(within(pool).getAllByText('Antonio')).toHaveLength(1); + expect(within(pool).getAllByText('Prospero')).toHaveLength(1); + }); + + it('lists a distractor of the same content as a paired option once in the pool', () => { + renderEditor({ + ...viewProps, + interaction: blockWithDecl(PAIRED_DISTRACTOR_XML, ONE_PAIR_DECL_XML), + }); + const pool = screen.getByRole('list', { name: tr.$tr('responsePoolLabel') }); + expect(within(pool).getAllByText('Antonio')).toHaveLength(1); + }); + + it('lists a blank option once per identifier in the pool', () => { + renderEditor({ + ...viewProps, + interaction: blockWithDecl(BLANK_CHOICES_XML, SHARED_CHOICE_DECL_XML), + }); + const pool = screen.getByRole('list', { name: tr.$tr('responsePoolLabel') }); + expect(within(pool).getAllByRole('listitem')).toHaveLength(4); + }); + + it('lists as many pool options as the saved XML holds once shared options are cleared', async () => { + const user = userEvent.setup(); + const { emitted, updateProps } = renderEditor({ + interaction: blockWithDecl(THRICE_SHARED_CHOICE_XML, THRICE_SHARED_CHOICE_DECL_XML), + }); + for (const number of [2, 3]) { + await user.click(button(editPairItem(number, 1))); + await user.clear(openTextarea()); + } + await updateProps({ mode: 'view' }); + const savedChoices = parseXML(latestBodyXml(emitted)).querySelectorAll( + 'qti-simple-associable-choice', + ); + const pool = screen.getByRole('list', { name: tr.$tr('responsePoolLabel') }); + expect(within(pool).getAllByRole('listitem')).toHaveLength(savedChoices.length); }); it('keeps the pool order across a round trip through edit mode', async () => { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/parse.js index 21d782ea8b..c2f6370882 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/parse.js @@ -4,7 +4,6 @@ import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration'; import { buildXmlNode, getContentHTML, getPromptHTML, parseXML } from '../../serialization/xml'; import CorrectResponse from '../../serialization/qti/declarations/correctResponse'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; -import { hasRichTextContent } from '../../utils/richText'; import { RESPONSE_IDENTIFIER } from '../../constants'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; @@ -116,28 +115,26 @@ export function parseAssociateInteraction(bodyXml, responseDeclarations) { } /** - * Serialize AssociateState → { bodyXml, responseDeclarations }. - * - * @param {AssociateState} state - * @param {string} _questionType - unused (associate has one question type); kept for API parity - * @param {object} declarationSchema - { baseType: string, cardinality: string } - * @returns {{ bodyXml: string, responseDeclarations: string[] }} + * @param {AssociateChoice[][]} pairs + * @param {AssociateChoice[]} distractors + * @returns {{ + * pairs: Array>, + * pool: Array, + * }} */ -export function buildAssociateInteractionXML(state, _questionType, declarationSchema) { - const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, pairs = [], distractors = [] } = state; - +export function resolveAssociateChoices(pairs, distractors) { const idByContent = new Map(); const contentById = new Map(); // Preserves the id a choice already carries unless it is already bound to // different content. function resolveChoice({ id, content }) { - const key = hasRichTextContent(content) ? richTextComparisonKey(content) : ''; + const key = richTextComparisonKey(content); // Blank choices are never deduped — a freshly added pair holds two of them, // and collapsing them would leave the pair unable to round-trip. if (key && idByContent.has(key)) { - return { id: idByContent.get(key), content }; + return { id: idByContent.get(key), content, key }; } const boundKey = contentById.get(id); @@ -147,7 +144,7 @@ export function buildAssociateInteractionXML(state, _questionType, declarationSc if (key) { idByContent.set(key, resolvedId); } - return { id: resolvedId, content }; + return { id: resolvedId, content, key }; } const resolvedPairs = pairs.map(pair => pair.map(resolveChoice)); @@ -155,20 +152,36 @@ export function buildAssociateInteractionXML(state, _questionType, declarationSc // Every appearance of a choice is one pairing it may take part in. const pool = new Map(); - for (const { id, content } of [...flatten(resolvedPairs), ...resolvedDistractors]) { + for (const { id, content, key } of [...flatten(resolvedPairs), ...resolvedDistractors]) { const entry = pool.get(id); if (entry) { entry.matchMax += 1; } else { - pool.set(id, { id, content, matchMax: 1 }); + pool.set(id, { id, content, key, matchMax: 1 }); } } + return { pairs: resolvedPairs, pool: [...pool.values()] }; +} + +/** + * Serialize AssociateState → { bodyXml, responseDeclarations }. + * + * @param {AssociateState} state + * @param {string} _questionType - unused (associate has one question type); kept for API parity + * @param {object} declarationSchema - { baseType: string, cardinality: string } + * @returns {{ bodyXml: string, responseDeclarations: string[] }} + */ +export function buildAssociateInteractionXML(state, _questionType, declarationSchema) { + const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, pairs = [], distractors = [] } = state; + + const { pairs: resolvedPairs, pool } = resolveAssociateChoices(pairs, distractors); + const children = []; if (prompt) { children.push(buildXmlNode({ tag: 'qti-prompt', innerHTML: prompt })); } - for (const { id, content, matchMax } of pool.values()) { + for (const { id, content, matchMax } of pool) { children.push( buildXmlNode({ tag: 'qti-simple-associable-choice', diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js index de35e5a22c..9db7d941a5 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js @@ -1,8 +1,10 @@ import flatten from 'lodash/flatten'; import { ValidationError } from '../../constants'; -import { hasRichTextContent } from '../../utils/richText'; import { duplicateDistractorErrors } from '../../utils/distractors'; -import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Validate AssociateState → ValidationError[]. @@ -23,13 +25,10 @@ export function validateAssociateInteraction(state) { errors.push({ code: ValidationError.PROMPT_REQUIRED }); } - // Every rule below asks the same two questions of a choice, and answering - // either one parses its HTML — so each choice is read once, here. - const read = ({ id, content }) => ({ - id, - filled: hasRichTextContent(content), - key: richTextComparisonKey(content), - }); + const read = ({ id, content }) => { + const key = richTextComparisonKey(content); + return { id, filled: Boolean(key), key }; + }; const readPairs = pairs.map(pair => pair.map(read)); const readDistractors = distractors.map(read); const allChoices = [...flatten(readPairs), ...readDistractors]; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/validation.js index e95154b237..6cb6bbe692 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/choice/validation.js @@ -1,6 +1,8 @@ import { QuestionType, ValidationError } from '../../constants'; -import { hasRichTextContent } from '../../utils/richText'; -import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Validate ChoiceState → ValidationError[]. @@ -23,12 +25,12 @@ export function validateChoiceInteraction(state, questionType) { const duplicateIds = new Set(); for (const choice of choices) { - if (!hasRichTextContent(choice.content)) { + const key = richTextComparisonKey(choice.content); + if (!key) { errors.push({ code: ValidationError.EMPTY_CHOICE_CONTENT, id: choice.id }); continue; } - const key = richTextComparisonKey(choice.content); if (firstSeenId.has(key)) { duplicateIds.add(firstSeenId.get(key)); duplicateIds.add(choice.id); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index ad5df9e584..08385dbfb4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -10,10 +10,12 @@ import { wrapInlineRuns, } from '../../serialization/xml'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; -import { hasRichTextContent } from '../../utils/richText'; import { RESPONSE_IDENTIFIER, SENTINEL_MARKER } from '../../constants'; import { _extractCorrectPairIds } from '../associate/parse'; -import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; const serializer = new XMLSerializer(); @@ -185,8 +187,8 @@ export function buildGapMatchInteractionXML(state, _questionType, declarationSch for (const gapEl of gapEls) { const gapId = claimExisting(gapEl.getAttribute('identifier'), 'gap'); const content = getContentHTML(gapEl); - if (hasRichTextContent(content)) { - const key = richTextComparisonKey(content); + const key = richTextComparisonKey(content); + if (key) { const ownAnswerId = gapEl.getAttribute(CORRECT_ATTR); const answerId = idByAnswerKey.get(key) || diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js index 05f9813ee2..e7c849374e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js @@ -1,5 +1,4 @@ import { ValidationError } from '../../constants'; -import { hasRichTextContent } from '../../utils/richText'; import { duplicateDistractorErrors } from '../../utils/distractors'; import { getGaps } from './parse'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; @@ -11,11 +10,10 @@ export function validateGapMatchInteraction(state) { return [{ code: ValidationError.NO_CORRECT_ANSWER }]; } - const read = (id, content) => ({ - id, - filled: hasRichTextContent(content), - key: richTextComparisonKey(content), - }); + const read = (id, content) => { + const key = richTextComparisonKey(content); + return { id, filled: Boolean(key), key }; + }; const readAnswers = gaps.map(({ id, answer }) => read(id, answer)); const readDistractors = distractors.map(({ id, content }) => read(id, content)); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue index 51af96f727..6153787ca5 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/Editor.vue @@ -141,12 +141,12 @@ import ValidationMessage from '../../components/ValidationMessage/index.vue'; import ClickableRegion from '../../components/ClickableRegion/index.vue'; import AnswerSettings from '../choice/components/AnswerSettings/index.vue'; - import { hasRichTextContent } from '../../utils/richText'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; import { useInlineChoicePassage } from './useInlineChoicePassage'; import InlineChoiceOptions from './components/InlineChoiceOptions/index.vue'; import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; + import { hasRichTextContent } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; export default { name: 'InlineChoiceEditor', diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue index d663179866..7f3bf55681 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/Editor.vue @@ -309,9 +309,12 @@ import ClickableRegion from '../../components/ClickableRegion/index.vue'; import EditableChipList from '../../components/EditableChipList/index.vue'; import ShuffledResponsePool from '../../components/ShuffledResponsePool/index.vue'; - import { hasRichTextContent } from '../../utils/richText'; + import { resolveMatchAnswers } from './parse'; import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; - import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + import { + hasRichTextContent, + richTextComparisonKey, + } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; /** Which card or chip list currently holds the open TipTap editor. */ @@ -663,18 +666,13 @@ const isOnlyRow = computed(() => state.value.rows.length <= 1); - // One option per association the learner can make: the emitted XML folds - // answers holding the same content into a single option whose match-max is - // the number of occurrences walked here. Blank answers are left unmarked: - // every blank keys the same, so they would mark an unwritten option correct. + // Kolibri's pool shows each choice of the response match set once. const poolChoices = computed(() => [ - ...state.value.rows.flatMap(row => - row.matches.map(choice => ({ - content: choice.content, - isCorrect: props.showAnswers && hasRichTextContent(choice.content), - })), - ), - ...state.value.distractors.map(choice => ({ content: choice.content, isCorrect: false })), + ...resolveMatchAnswers(state.value.rows).answers.map(({ content, key }) => ({ + content, + isCorrect: props.showAnswers && Boolean(key), + })), + ...state.value.distractors.map(({ content }) => ({ content, isCorrect: false })), ]); return { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js index 423d9b3d7d..1ebbe71224 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/__tests__/Editor.spec.js @@ -77,6 +77,17 @@ const SHARED_ANSWER_DECL_XML = ` `; +const ANSWER_AS_DISTRACTOR_XML = ` + Match each animal to its class. + + Dog + + + Mammal + Mammal + +`; + const BLANK_ANSWER_XML = ` Match each animal to its class. @@ -87,6 +98,25 @@ const BLANK_ANSWER_XML = ` `; +const TWO_BLANK_ANSWERS_XML = ` + Match each animal to its class. + + Dog + Cat + + + + + +`; + +const TWO_BLANK_ANSWERS_DECL_XML = ` + + row_dog choice_dog + row_cat choice_cat + +`; + // The mock TipTapEditor renders a only for the card currently open. const openTextareas = () => screen.queryAllByRole('textbox').filter(el => el.tagName === 'TEXTAREA'); @@ -808,14 +838,39 @@ describe('MatchEditor', () => { const renderView = (props = {}) => renderEditor({ mode: 'view', ...props }); const pool = () => screen.getByRole('list', { name: tr.$tr('responsePoolLabel') }); - it('repeats an answer in the pool once per row it serves, and each distractor once', () => { + it('lists an answer shared by several rows once in the pool, and each distractor once', () => { renderView({ interaction: blockWithDecl(SHARED_ANSWER_XML, SHARED_ANSWER_DECL_XML), }); - expect(within(pool()).getAllByText('Mammal')).toHaveLength(2); + expect(within(pool()).getAllByText('Mammal')).toHaveLength(1); expect(within(pool()).getAllByText('Reptile')).toHaveLength(1); }); + it('lists a distractor apart from an answer of the same content', () => { + renderView({ + interaction: blockWithDecl(ANSWER_AS_DISTRACTOR_XML, ONE_ROW_DECL_XML), + }); + expect(within(pool()).getAllByText('Mammal')).toHaveLength(2); + }); + + it('paints the one pool chip of an answer shared by several rows green', () => { + const palette = themePalette(); + renderView({ + interaction: blockWithDecl(SHARED_ANSWER_XML, SHARED_ANSWER_DECL_XML), + showAnswers: true, + }); + const chips = within(pool()).getAllByText('Mammal'); + expect(chips).toHaveLength(1); + expect(chips[0].closest('li')).toHaveStyle({ borderColor: palette.green.v_600 }); + }); + + it('lists each blank answer as its own pool chip', () => { + renderView({ + interaction: blockWithDecl(TWO_BLANK_ANSWERS_XML, TWO_BLANK_ANSWERS_DECL_XML), + }); + expect(within(pool()).getAllByRole('listitem')).toHaveLength(2); + }); + it('lists every row prompt and no answer when answers are hidden', () => { renderView(); for (const content of ['Dog', 'Eagle', 'Frog']) { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js index c498bc8e4a..4e81b80576 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/parse.js @@ -2,7 +2,6 @@ import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration'; import { buildXmlNode, getContentHTML, getPromptHTML, parseXML } from '../../serialization/xml'; import CorrectResponse from '../../serialization/qti/declarations/correctResponse'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; -import { hasRichTextContent } from '../../utils/richText'; import { RESPONSE_IDENTIFIER } from '../../constants'; import { _extractCorrectPairIds, newChoice } from '../associate/parse'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; @@ -124,36 +123,32 @@ export function parseMatchInteraction(bodyXml, responseDeclarations) { }; } -/** - * Serialize MatchState → { bodyXml, responseDeclarations }. - * - * @param {MatchState} state - * @param {string} _questionType - unused (match has one question type); kept for API parity - * @param {object} declarationSchema - { baseType: string, cardinality: string } - * @returns {{ bodyXml: string, responseDeclarations: string[] }} - */ -export function buildMatchInteractionXML(state, _questionType, declarationSchema) { - const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, rows = [], distractors = [] } = state; - - // Ids are unique across both match sets; the first appearance keeps its id. +// Ids are unique across both match sets; the first appearance keeps its id. +function idClaimer() { const usedIds = new Set(); - function claim(id, prefix) { + return (id, prefix) => { const resolved = id && !usedIds.has(id) ? id : generateRandomSlug(prefix); usedIds.add(resolved); return resolved; - } - - // Rows and distractors are never merged, so any repeat of their id is a conflict. - const rowIds = rows.map(row => claim(row.id, 'row')); + }; +} +/** + * @param {MatchRow[]} rows + * @param {function(string, string): string} [claim] + * @returns {{ + * answers: Array, + * answerIds: string[][], + * }} + */ +export function resolveMatchAnswers(rows, claim = idClaimer()) { // Answers of equal content are one choice, serving every row that names it; // blanks never are. const idByAnswerKey = new Map(); const answers = new Map(); - const correctPairs = []; - rows.forEach((row, index) => { - for (const { id, content } of row.matches) { - const key = hasRichTextContent(content) ? richTextComparisonKey(content) : ''; + const answerIds = rows.map(row => + row.matches.map(({ id, content }) => { + const key = richTextComparisonKey(content); const resolved = (key && idByAnswerKey.get(key)) || claim(id, 'choice'); if (key) idByAnswerKey.set(key, resolved); @@ -161,11 +156,32 @@ export function buildMatchInteractionXML(state, _questionType, declarationSchema if (answer) { answer.matchMax += 1; } else { - answers.set(resolved, { id: resolved, content, matchMax: 1 }); + answers.set(resolved, { id: resolved, content, key, matchMax: 1 }); } - correctPairs.push([rowIds[index], resolved]); - } - }); + return resolved; + }), + ); + return { answers: [...answers.values()], answerIds }; +} + +/** + * Serialize MatchState → { bodyXml, responseDeclarations }. + * + * @param {MatchState} state + * @param {string} _questionType - unused (match has one question type); kept for API parity + * @param {object} declarationSchema - { baseType: string, cardinality: string } + * @returns {{ bodyXml: string, responseDeclarations: string[] }} + */ +export function buildMatchInteractionXML(state, _questionType, declarationSchema) { + const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, rows = [], distractors = [] } = state; + + const claim = idClaimer(); + + // Rows and distractors are never merged, so any repeat of their id is a conflict. + const rowIds = rows.map(row => claim(row.id, 'row')); + + const { answers, answerIds } = resolveMatchAnswers(rows, claim); + const correctPairs = answerIds.flatMap((ids, index) => ids.map(id => [rowIds[index], id])); const distractorChoices = distractors.map(({ id, content }) => ({ id: claim(id, 'choice'), @@ -188,7 +204,7 @@ export function buildMatchInteractionXML(state, _questionType, declarationSchema }); const responseSet = buildXmlNode({ tag: 'qti-simple-match-set', - children: [...answers.values(), ...distractorChoices].map(choiceNode), + children: [...answers, ...distractorChoices].map(choiceNode), }); const children = []; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js index a7ac4d0037..57577f2f6a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js @@ -1,7 +1,9 @@ import { ValidationError } from '../../constants'; -import { hasRichTextContent } from '../../utils/richText'; import { duplicateDistractorErrors } from '../../utils/distractors'; -import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Validate MatchState → ValidationError[]. @@ -21,13 +23,10 @@ export function validateMatchInteraction(state) { errors.push({ code: ValidationError.PROMPT_REQUIRED }); } - // Every rule below asks the same two questions of a choice, and answering - // either one parses its HTML — so each choice is read once, here. - const read = ({ id, content }) => ({ - id, - filled: hasRichTextContent(content), - key: richTextComparisonKey(content), - }); + const read = ({ id, content }) => { + const key = richTextComparisonKey(content); + return { id, filled: Boolean(key), key }; + }; const readRows = rows.map(row => ({ ...read(row), matches: row.matches.map(read) })); const readDistractors = distractors.map(read); const readChoices = [...readRows.flatMap(row => row.matches), ...readDistractors]; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/validation.js index 4f60436825..66e692a523 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/ordering/validation.js @@ -1,6 +1,8 @@ import { ValidationError } from '../../constants'; -import { hasRichTextContent } from '../../utils/richText'; -import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; +import { + hasRichTextContent, + richTextComparisonKey, +} from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Validate OrderingState → ValidationError[]. @@ -24,12 +26,12 @@ export function validateOrderingInteraction(state) { const duplicateIds = new Set(); for (const item of items) { - if (!hasRichTextContent(item.content)) { + const key = richTextComparisonKey(item.content); + if (!key) { errors.push({ code: ValidationError.EMPTY_CHOICE_CONTENT, id: item.id }); continue; } - const key = richTextComparisonKey(item.content); if (firstSeenId.has(key)) { duplicateIds.add(firstSeenId.get(key)); duplicateIds.add(item.id); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js index 75ae36a06c..86f82c75d2 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js @@ -41,7 +41,7 @@ describe('validateTextEntryInteraction', () => { it('returns PROMPT_REQUIRED for a visually-empty -only prompt (entity regression)', () => { // The naive /<[^>]*>/g regex leaves the literal text " " which is truthy, - // so PROMPT_REQUIRED would silently pass. QTISanitizer.stripTags parses via + // so PROMPT_REQUIRED would silently pass. hasRichTextContent parses via // DOMParser text/html, which decodes entities and returns actual whitespace. const errors = validateTextEntryInteraction( { ...VALID_NUMERIC_STATE, prompt: ' ' }, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js index 14e30088d6..bd2780baa6 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js @@ -1,7 +1,7 @@ import { QuestionType, ValidationError } from '../../constants'; import { parseXsdDouble } from '../../utils/math'; import { readLocaleNumber } from '../../utils/localeNumbers'; -import { hasRichTextContent } from '../../utils/richText'; +import { hasRichTextContent } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** * Validate TextEntryState → ValidationError[]. diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/hints.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/hints.spec.js index aa2212bf29..7f32b4aeb8 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/hints.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/hints.spec.js @@ -54,7 +54,7 @@ describe('hintHasContent', () => { }); // A hint can be entirely an image or a formula, which hasRichTextContent covers in - // full — see utils/__tests__/richText.spec.js. + // full — see TipTapEditor/__tests__/hasRichTextContent.spec.js. it('is true for a hint that is nothing but an image', () => { expect(hintHasContent({ content: '' })).toBe(true); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js index b9f43d529e..50d20efddb 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js @@ -15,7 +15,6 @@ */ import { generateRandomSlug } from '../utils/generateRandomSlug'; -import { hasRichTextContent } from '../utils/richText'; import { attributesOf, contentChildrenOf, @@ -23,6 +22,7 @@ import { hasNonNamespaceAttributes, isChildElement, } from './xml'; +import { hasRichTextContent } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** The catalog this editor writes hints into. */ export const HINT_CATALOG_ID = 'kolibri-hints'; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js deleted file mode 100644 index 5f66ff7c9e..0000000000 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js +++ /dev/null @@ -1,130 +0,0 @@ -/** - * QTISanitizer — input validation and sanitization at every data entry boundary. - * - * @module serialization/qti/QTISanitizer - */ - -import { parseXML } from '../xml'; - -// Valid QTI 3.0 base-type values — https://www.imsglobal.org/spec/qti/v3p0/impl/#h.wq4e8lbs4wa9 -const VALID_BASE_TYPES = new Set([ - 'boolean', - 'directedPair', - 'duration', - 'file', - 'float', - 'identifier', - 'integer', - 'pair', - 'point', - 'string', - 'uri', -]); - -// Valid QTI 3.0 cardinality values — https://www.imsglobal.org/spec/qti/v3p0/impl/#h.wq4e8lbs4wa9 -const VALID_CARDINALITIES = new Set(['single', 'multiple', 'ordered', 'record']); - -// QTI identifier pattern — Unicode-aware NCName (\w is ASCII-only in JS) -const IDENTIFIER_RE = /^[\p{L}_][\p{L}\p{N}_.-]*$/u; - -export class QTISanitizer { - /** - * Validate a QTI base-type string. - * Throws if the value is not in the spec-defined set. - * - * @param {string} baseType - * @returns {string} The same value if valid - * @throws {Error} - */ - static validateBaseType(baseType) { - if (baseType === null || baseType === undefined) return baseType; - if (!VALID_BASE_TYPES.has(baseType)) { - throw new Error( - `QTISanitizer: invalid base-type "${baseType}". ` + - `Must be one of: ${[...VALID_BASE_TYPES].join(', ')}.`, - ); - } - return baseType; - } - - /** - * Validate a QTI cardinality string. - * Throws if the value is not in the spec-defined set. - * - * @param {string} cardinality - * @returns {string} The same value if valid - * @throws {Error} - */ - static validateCardinality(cardinality) { - if (!VALID_CARDINALITIES.has(cardinality)) { - throw new Error( - `QTISanitizer: invalid cardinality "${cardinality}". ` + - `Must be one of: ${[...VALID_CARDINALITIES].join(', ')}.`, - ); - } - return cardinality; - } - - /** - * Validate a QTI identifier (NCName-based). - * Throws if the value does not match the QTI identifier pattern. - * - * @param {string} identifier - * @returns {string} The same value if valid - * @throws {Error} - */ - static validateIdentifier(identifier) { - if (!identifier || !IDENTIFIER_RE.test(identifier)) { - throw new Error( - `QTISanitizer: invalid QTI identifier "${identifier}". ` + - `Must start with a letter or underscore and contain only word characters, hyphens, or dots.`, - ); - } - return identifier; - } - - /** - * Strip HTML tags from a string value. - * - * @param {string} value - Raw string (identifier, float string, etc.) - * @returns {string} Value with all HTML tags stripped and text content extracted - */ - static stripTags(value) { - if (typeof value !== 'string') return String(value ?? ''); - // Fast path: with no `<` or `&` there is nothing to strip or decode. - if (!value.includes('<') && !value.includes('&')) return value; - const doc = parseXML(value, 'text/html'); - // Remove script and style elements entirely — we do NOT want their text content. - doc.querySelectorAll('script, style').forEach(el => el.remove()); - return doc.body.textContent ?? ''; - } - - /** - * Parse a string as a float and validate it is a finite number. - * Throws if the value is not a valid finite number. - * - * @param {string|number} value - * @param {string} [fieldName] - Label for error messages - * @returns {number} - * @throws {Error} - */ - static parseFloat(value, fieldName = 'value') { - const n = Number(value); - if (!isFinite(n)) { - throw new Error(`QTISanitizer: "${fieldName}" must be a finite number, got "${value}".`); - } - return n; - } - - /** - * Sanitize an array of string values (e.g. correctResponse, defaultValue). - * Trims whitespace and strips HTML tags from each entry. - * Removes entries that are empty after sanitization. - * - * @param {string[]} values - * @returns {string[]} - */ - static sanitizeValues(values) { - return values.map(v => QTISanitizer.stripTags(String(v).trim())).filter(v => v.length > 0); - } -} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js deleted file mode 100644 index ccb797803a..0000000000 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js +++ /dev/null @@ -1,177 +0,0 @@ -import { QTISanitizer } from '../QTISanitizer.js'; - -describe('QTISanitizer.validateBaseType', () => { - it.each([ - 'boolean', - 'float', - 'identifier', - 'integer', - 'string', - 'point', - 'pair', - 'directedPair', - 'duration', - 'file', - 'uri', - ])('accepts valid base-type "%s"', bt => expect(QTISanitizer.validateBaseType(bt)).toBe(bt)); - - it('throws for an unknown base-type', () => { - expect(() => QTISanitizer.validateBaseType('foo')).toThrow('invalid base-type'); - }); - - it('throws for an empty string', () => { - expect(() => QTISanitizer.validateBaseType('')).toThrow(); - }); - - it('returns null/undefined unchanged (base-type is optional on outcome declarations)', () => { - expect(QTISanitizer.validateBaseType(null)).toBeNull(); - expect(QTISanitizer.validateBaseType(undefined)).toBeUndefined(); - }); -}); - -describe('QTISanitizer.validateCardinality', () => { - it.each(['single', 'multiple', 'ordered', 'record'])('accepts valid cardinality "%s"', c => - expect(QTISanitizer.validateCardinality(c)).toBe(c), - ); - - it('throws for an unknown cardinality', () => { - expect(() => QTISanitizer.validateCardinality('many')).toThrow('invalid cardinality'); - }); - - it('throws for an empty string', () => { - expect(() => QTISanitizer.validateCardinality('')).toThrow(); - }); -}); - -describe('QTISanitizer.validateIdentifier', () => { - it('accepts a plain ASCII identifier', () => { - expect(QTISanitizer.validateIdentifier('RESPONSE')).toBe('RESPONSE'); - }); - - it('accepts identifiers starting with underscore', () => { - expect(QTISanitizer.validateIdentifier('_choice1')).toBe('_choice1'); - }); - - it('accepts identifiers with hyphens and dots', () => { - expect(QTISanitizer.validateIdentifier('choice-A.1')).toBe('choice-A.1'); - }); - - it('accepts non-ASCII Unicode identifiers (i18n item banks)', () => { - expect(QTISanitizer.validateIdentifier('RÉSPONSE')).toBe('RÉSPONSE'); - }); - - it('throws for an identifier starting with a digit', () => { - expect(() => QTISanitizer.validateIdentifier('1choice')).toThrow('invalid QTI identifier'); - }); - - it('throws for an identifier containing spaces', () => { - expect(() => QTISanitizer.validateIdentifier('choice A')).toThrow(); - }); - - it('throws for an empty string', () => { - expect(() => QTISanitizer.validateIdentifier('')).toThrow(); - }); - - it('throws for an identifier that is just a number', () => { - expect(() => QTISanitizer.validateIdentifier('123')).toThrow(); - }); -}); - -describe('QTISanitizer.stripTags — XSS defence', () => { - it('passes through a plain string unchanged', () => { - expect(QTISanitizer.stripTags('ChoiceA')).toBe('ChoiceA'); - }); - - it('strips a simple ')).toBe(''); - }); - - it('strips HTML bold tags and keeps text content', () => { - expect(QTISanitizer.stripTags('bold')).toBe('bold'); - }); - - it('strips nested tags', () => { - expect(QTISanitizer.stripTags('inner')).toBe('inner'); - }); - - it('handles img onerror XSS payload', () => { - const payload = ''; - // After stripping tags, no html remains — textContent of is empty - expect(QTISanitizer.stripTags(payload)).toBe(''); - }); - - it('preserves non-ASCII text (i18n)', () => { - expect(QTISanitizer.stripTags('选择甲')).toBe('选择甲'); - }); - - it('preserves numbers', () => { - expect(QTISanitizer.stripTags('3.14')).toBe('3.14'); - }); - - it('preserves ampersand entity text (not HTML)', () => { - // A raw `&` without a tag — not HTML injection, passes through - expect(QTISanitizer.stripTags('A & B')).toBe('A & B'); - }); - - it('decodes entities in a fragment without tags', () => { - expect(QTISanitizer.stripTags(' A & B')).toBe(' A & B'); - }); - - it('handles non-string input by coercing to string', () => { - expect(QTISanitizer.stripTags(42)).toBe('42'); - expect(QTISanitizer.stripTags(null)).toBe(''); - }); -}); - -describe('QTISanitizer.parseFloat', () => { - it('parses a string float', () => { - expect(QTISanitizer.parseFloat('3.14')).toBe(3.14); - }); - - it('parses a string integer', () => { - expect(QTISanitizer.parseFloat('0')).toBe(0); - }); - - it('parses a negative number', () => { - expect(QTISanitizer.parseFloat('-0.5')).toBe(-0.5); - }); - - it('passes through a number', () => { - expect(QTISanitizer.parseFloat(1)).toBe(1); - }); - - it('throws for NaN input', () => { - expect(() => QTISanitizer.parseFloat('not-a-number')).toThrow('finite number'); - }); - - it('throws for Infinity', () => { - expect(() => QTISanitizer.parseFloat(Infinity)).toThrow(); - }); - - it('includes the field name in the error', () => { - expect(() => QTISanitizer.parseFloat('x', 'mapped-value')).toThrow('mapped-value'); - }); -}); - -describe('QTISanitizer.sanitizeValues', () => { - it('trims whitespace from each entry', () => { - expect(QTISanitizer.sanitizeValues([' A ', 'B'])).toEqual(['A', 'B']); - }); - - it('strips HTML tags from each entry', () => { - expect(QTISanitizer.sanitizeValues(['Choice', 'B'])).toEqual(['Choice', 'B']); - }); - - it('removes entries that are empty after sanitization', () => { - expect(QTISanitizer.sanitizeValues(['', 'Valid'])).toEqual(['Valid']); - }); - - it('returns an empty array for an all-empty input', () => { - expect(QTISanitizer.sanitizeValues(['', ' '])).toEqual([]); - }); - - it('preserves i18n values', () => { - expect(QTISanitizer.sanitizeValues(['选择甲', 'اختيار'])).toEqual(['选择甲', 'اختيار']); - }); -}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/richText.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/richText.js deleted file mode 100644 index 9ead9a85fe..0000000000 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/richText.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * What the item's rich text editors hold, and whether any of it says anything. - * - * Every TipTap editor in this editor — a prompt, a choice, an ordering item, a hint — - * holds an HTML fragment, and such a fragment is empty in more ways than an empty - * string: a `` the author opened and never typed into, a paragraph holding - * nothing but ` `. - * - * It can also say something without saying it in words. An image, or a formula from the - * editor's own formula button, is the whole content — a converted legacy question often - * asks its question entirely in a picture. Reading only the text would call those empty, - * dropping a hint on the next save and failing a question that has nothing wrong with it. - */ - -import { QTISanitizer } from '../serialization/qti/QTISanitizer'; -import { parseXML } from '../serialization/xml'; - -/** - * Markup that is content in its own right, with no text to find. - * - * A formula is authored as the `data-latex` span the editor's Math node round-trips, and - * only becomes MathML when the item is published — so both forms count, the item being - * read here having come from either side of that. - */ -const EMBEDDED_MEDIA = 'img, math, svg, span[data-latex]'; - -/** - * The fragment's visible text. - * - * Parsed rather than pattern-stripped, so entities are decoded — ` ` is whitespace - * the trim removes, not the literal six characters a `/<[^>]*>/g` strip leaves behind. - * - * @param {string} content - * @returns {string} - */ -function toText(content) { - return QTISanitizer.stripTags(content ?? '').trim(); -} - -/** - * Whether a rich text fragment holds anything worth keeping. - * - * @param {string} content - HTML fragment from a TipTap editor - * @returns {boolean} - */ -export function hasRichTextContent(content) { - return ( - toText(content).length > 0 || - Boolean(parseXML(content ?? '', 'text/html').querySelector(EMBEDDED_MEDIA)) - ); -} diff --git a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey.js b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey.js index e23c2cfed2..f04d0c9f02 100644 --- a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey.js +++ b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey.js @@ -14,6 +14,47 @@ const UNPARSED_MEDIA = 'svg, math'; // Marks that change what text says. const SCRIPTS = ['superscript', 'subscript']; +/** + * Markup that is content in its own right, with no text to find. + * + * A formula is authored as the `data-latex` span the editor's Math node round-trips, and + * only becomes MathML when the item is published — so both forms count, the item being + * read here having come from either side of that. + */ +const EMBEDDED_MEDIA = 'img, math, svg, span[data-latex]'; + +function parseHTML(html) { + return new DOMParser().parseFromString(html ?? '', 'text/html'); +} + +function toText(document) { + // Remove script and style elements entirely — we do NOT want their text content. + document.querySelectorAll('script, style').forEach(el => el.remove()); + return document.body.textContent.trim(); +} + +function hasContent(document) { + return Boolean(document.querySelector(EMBEDDED_MEDIA)) || toText(document).length > 0; +} + +/** + * Whether a rich text fragment holds anything worth keeping. + * + * A fragment is empty in more ways than an empty string: a `` the author opened + * and never typed into, a paragraph holding nothing but ` `. + * + * It can also say something without saying it in words. An image, or a formula from the + * editor's own formula button, is the whole content — a converted legacy question often + * asks its question entirely in a picture. Reading only the text would call those empty, + * dropping a hint on the next save and failing a question that has nothing wrong with it. + * + * @param {string} content - HTML fragment from a TipTap editor + * @returns {boolean} + */ +export function hasRichTextContent(content) { + return hasContent(parseHTML(content)); +} + /** * A key two fragments can be compared on to tell whether they say the same thing. * @@ -22,10 +63,11 @@ const SCRIPTS = ['superscript', 'subscript']; * re-render keys the same, unless it has SVG or MathML, which TipTap drops. * * @param {string} html - HTML fragment from a TipTap editor - * @returns {string} + * @returns {string} '' for a fragment without rich text content */ export function richTextComparisonKey(html) { - const document = new DOMParser().parseFromString(html ?? '', 'text/html'); + const document = parseHTML(html); + if (!hasContent(document)) return ''; // A formula stands in for each, so parsing keeps its place. HTML parsing turns NUL // into U+FFFD, so no stored formula has this LaTeX. const unparsed = new Map(); diff --git a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/comparisonKey.spec.js b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/comparisonKey.spec.js index fbf0aa4e3d..119be0ba99 100644 --- a/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/comparisonKey.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/comparisonKey.spec.js @@ -2,6 +2,13 @@ import { useEditor } from '../TipTapEditor/composables/useEditor'; import { richTextComparisonKey } from '../TipTapEditor/utils/comparisonKey'; describe('richTextComparisonKey', () => { + it.each(['', null, '', ' ', ''])( + 'is empty for %s, which holds no rich text content', + blank => { + expect(richTextComparisonKey(blank)).toBe(''); + }, + ); + it.each([ ['bold', 'x2'], ['italic', 'x2'], diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/hasRichTextContent.spec.js similarity index 89% rename from contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js rename to contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/hasRichTextContent.spec.js index 31ef06600e..5ec8766996 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/TipTapEditor/__tests__/hasRichTextContent.spec.js @@ -1,4 +1,4 @@ -import { hasRichTextContent } from '../richText'; +import { hasRichTextContent } from '../TipTapEditor/utils/comparisonKey'; describe('hasRichTextContent', () => { it.each([ @@ -12,6 +12,8 @@ describe('hasRichTextContent', () => { // The literal six characters a pattern strip leaves behind are not content. [' ', false], [' ', false], + ['', false], + ['', false], // A prompt, choice or hint can be entirely an image or a formula — from a converted // legacy question, or from the editor's own image and formula buttons. ['', true],
Match each character to his adversary.
Match each animal to its class.
x2