')),
+ ).toBe(false);
+ });
+
+ it('accepts whitespace and comments around it', () => {
+ const [{ bodyXml }] = interactionsOf(VALID_CHOICE_ITEM_DOCUMENT);
+ expect(
+ isSupported(withBody(VALID_CHOICE_ITEM_DOCUMENT, `\n \n${bodyXml}\n`)),
+ ).toBe(true);
+ });
+ });
+
+ describe('body content around a text entry', () => {
+ it.each([
+ ['a paragraph shared with other content', TEXT_ENTRY_ITEM_DOCUMENT_SHARED_PARAGRAPH],
+ [
+ 'a paragraph shared with CDATA text',
+ FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ '
', '/>More.
'),
+ ],
+ [
+ 'content after the wrapper of its paragraph',
+ CONVERTED_TEXT_ENTRY_DOCUMENT.replace('/>', '/>More.
'),
+ ],
+ [
+ 'no paragraph of its own',
+ FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ /]*)\/><\/p>/,
+ '',
+ ),
+ ],
+ [
+ 'a paragraph with an attribute',
+ FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ '| Q? | ${TEXT_ENTRY_PARAGRAPH} |
`,
+ ),
+ ],
+ [
+ 'a paragraph inside a list',
+ withBody(
+ FREE_RESPONSE_ITEM_DOCUMENT,
+ `- Q?
- ${TEXT_ENTRY_PARAGRAPH}
`,
+ ),
+ ],
+ [
+ 'a paragraph inside a wrapper with an attribute',
+ CONVERTED_TEXT_ENTRY_DOCUMENT.replace('', '
'),
+ ],
+ [
+ 'a wrapper after the prompt',
+ withBody(FREE_RESPONSE_ITEM_DOCUMENT, `
Q?
${TEXT_ENTRY_PARAGRAPH}
`),
+ ],
+ ])('rejects %s', (_, document) => {
+ expect(isSupported(document)).toBe(false);
+ });
+
+ it.each([
+ [
+ 'its paragraph',
+ FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ '
',
+ '',
+ ),
+ ],
+ ])('accepts namespace declarations on %s', (_, document) => {
+ expect(isSupported(document)).toBe(true);
+ });
+ });
+
+ describe('attributes on the item body', () => {
+ it.each([
+ ['dir', 'dir="rtl"'],
+ ['xml:lang', 'xml:lang="ar"'],
+ ['class', 'class="x"'],
+ ])('rejects a choice item whose body has %s', (_, attribute) => {
+ expect(
+ isSupported(
+ VALID_CHOICE_ITEM_DOCUMENT.replace('
', ``),
+ ),
+ ).toBe(false);
+ });
+
+ it('rejects a text entry item whose body has an attribute', () => {
+ expect(
+ isSupported(
+ FREE_RESPONSE_ITEM_DOCUMENT.replace('', ''),
+ ),
+ ).toBe(false);
+ });
+
+ it('accepts namespace declarations on the body', () => {
+ expect(
+ isSupported(
+ VALID_CHOICE_ITEM_DOCUMENT.replace(
+ '',
+ '',
+ ),
+ ),
+ ).toBe(true);
+ });
+ });
+
+ describe('an item the editor has saved', () => {
+ const textEntryAfter = prompt =>
+ withBody(FREE_RESPONSE_ITEM_DOCUMENT, `${prompt}${TEXT_ENTRY_PARAGRAPH}`);
+
+ it.each([
+ ['a choice interaction', VALID_CHOICE_ITEM_DOCUMENT],
+ ['an ordering interaction', ORDERING_ITEM_DOCUMENT_NO_PROMPT],
+ ['an associate interaction', VALID_ASSOCIATE_ITEM_DOCUMENT],
+ ['a match interaction', VALID_MATCH_ITEM_DOCUMENT],
+ ['a text entry after a plain prompt', FREE_RESPONSE_ITEM_DOCUMENT],
+ ['a text entry with no prompt', textEntryAfter('')],
+ ['a text entry inside the converted wrapper', CONVERTED_TEXT_ENTRY_DOCUMENT],
+ ['a text entry after an image', textEntryAfter('
')],
+ [
+ 'a text entry after MathML',
+ textEntryAfter(''),
+ ],
+ ['a text entry after a right-to-left prompt', textEntryAfter('Q?
')],
+ ['a text entry after a list', textEntryAfter('')],
+ ])('stays editable with %s, after one save and two', (_, document) => {
+ const once = save(document);
+ expect(isSupported(once)).toBe(true);
+ expect(isSupported(save(once))).toBe(true);
+ });
});
});
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js
index de5a8aa17b..49f02599e8 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js
@@ -1,4 +1,4 @@
-import { parseXML } from '../serialization/xml';
+import { hasNonNamespaceAttributes, parseXML } from '../serialization/xml';
import { ValidationError } from '../constants';
import { HEADLESS_INTERACTIONS, SINGLE_INSTANCE_INTERACTIONS, descriptors } from './descriptors';
@@ -39,25 +39,46 @@ export function resolveDescriptor(bodyXml, responseDeclarations) {
}
/**
- * Whether the editor can edit an item faithfully: it holds exactly one interaction block
- * (one interaction, for descriptors in SINGLE_INSTANCE_INTERACTIONS), and a descriptor
+ * The descriptor that can edit an item's interaction: the item holds exactly one interaction
+ * block (one interaction, for descriptors in SINGLE_INSTANCE_INTERACTIONS), and a descriptor
* with an editor exists for it that could read the interaction.
*
- * @param {Array<{ bodyXml: string, responseDeclarations: string[] }>} interactions - As
- * returned by parseItem
- * @returns {boolean}
+ * @param {Array<{ bodyXml: string, responseDeclarations: string[] }>} interactions
+ * @returns {object|null}
*/
-export function isSupportedItem(interactions) {
+function editableDescriptor(interactions) {
if (interactions.length !== 1) {
- return false;
+ return null;
}
const [{ bodyXml, responseDeclarations }] = interactions;
const { descriptor, error } = resolveDescriptor(bodyXml, responseDeclarations);
if (descriptor === null || error || HEADLESS_INTERACTIONS.includes(descriptor.type)) {
+ return null;
+ }
+ if (
+ SINGLE_INSTANCE_INTERACTIONS.includes(descriptor.type) &&
+ parseXML(bodyXml).getElementsByTagName(descriptor.type).length > 1
+ ) {
+ return null;
+ }
+ return descriptor;
+}
+
+/**
+ * Whether the editor can edit an item faithfully: its interaction is supported, and the body
+ * has the shape that interaction's builder writes.
+ *
+ * @param {Array<{ bodyXml: string, responseDeclarations: string[] }>} interactions - As
+ * returned by parseItem
+ * @param {string} itemBodyXml - The item's ``, as returned by parseItem
+ * @returns {boolean}
+ */
+export function isSupportedItem(interactions, itemBodyXml) {
+ const descriptor = editableDescriptor(interactions);
+ if (descriptor === null) {
return false;
}
- return (
- !SINGLE_INSTANCE_INTERACTIONS.includes(descriptor.type) ||
- parseXML(bodyXml).getElementsByTagName(descriptor.type).length <= 1
- );
+ const bodyEl = parseXML(itemBodyXml).documentElement;
+ // The builders write a bare , so any attribute would be dropped.
+ return !hasNonNamespaceAttributes(bodyEl) && descriptor.isSupportedBody(bodyEl);
}
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js
index 934189eea7..f411f39f74 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/Descriptor.js
@@ -1,7 +1,11 @@
import { QtiInteraction, QuestionType, BaseType, Cardinality, Placement } from '../../constants';
import { parseXML } from '../../serialization/xml';
import { InteractionDescriptor } from '../InteractionDescriptor';
-import { parseTextEntryInteraction, buildTextEntryInteractionXML } from './parse';
+import {
+ parseTextEntryInteraction,
+ buildTextEntryInteractionXML,
+ isSupportedTextEntryBody,
+} from './parse';
import { validateTextEntryInteraction } from './validation';
/**
@@ -46,6 +50,11 @@ class TextEntryInteractionDescriptor extends InteractionDescriptor {
return !!el.querySelector(QtiInteraction.TEXT_ENTRY);
}
+ /** @param {Element} bodyEl */
+ isSupportedBody(bodyEl) {
+ return isSupportedTextEntryBody(bodyEl);
+ }
+
/**
* Reads base-type from the response declaration to determine question type.
*
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js
index 25bcf89cde..af520ec309 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js
@@ -1,5 +1,12 @@
import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration';
-import { buildXmlNode, parseXML, serializeAsHtml, wrapInlineRuns } from '../../serialization/xml';
+import {
+ buildXmlNode,
+ hasNonNamespaceAttributes,
+ isContentNode,
+ parseXML,
+ serializeAsHtml,
+ wrapInlineRuns,
+} from '../../serialization/xml';
import CorrectResponse from '../../serialization/qti/declarations/correctResponse';
import Mapping from '../../serialization/qti/declarations/mapping';
import { generateRandomSlug } from '../../utils/generateRandomSlug';
@@ -44,6 +51,48 @@ export function _defaultState() {
};
}
+function contentOf(el) {
+ return [...el.childNodes].filter(isContentNode);
+}
+
+function hasContentAfter(node) {
+ for (let sibling = node.nextSibling; sibling; sibling = sibling.nextSibling) {
+ if (isContentNode(sibling)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/**
+ * Whether the item body has the shape buildTextEntryInteractionXML writes: the prompt, then a
+ * `` holding only the interaction. Anything else would be dropped from the interaction's
+ * `
`, or moved ahead of it if it follows. Legacy conversion wraps the same shape in a bare
+ * `
`, the body's only content.
+ *
+ * @param {Element} bodyEl - The `
` element
+ * @returns {boolean}
+ */
+export function isSupportedTextEntryBody(bodyEl) {
+ const paragraph = bodyEl.querySelector('qti-text-entry-interaction').parentElement;
+ if (
+ paragraph.localName !== 'p' ||
+ hasNonNamespaceAttributes(paragraph) ||
+ contentOf(paragraph).length > 1 ||
+ hasContentAfter(paragraph)
+ ) {
+ return false;
+ }
+ const container = paragraph.parentElement;
+ return (
+ container === bodyEl ||
+ (container.localName === 'div' &&
+ !hasNonNamespaceAttributes(container) &&
+ container.parentElement === bodyEl &&
+ contentOf(bodyEl).length === 1)
+ );
+}
+
/**
* Serializes the body element children to an HTML string, excluding the
* element that directly contains the ``.
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js
index 271700c0f6..9881ac94e1 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js
@@ -13,6 +13,7 @@ import fs from 'fs';
import path from 'path';
import { parseItem } from '../parseItem';
import { assembleItemXml } from '../assembleItem';
+import { isSupportedItem } from '../../interactions/resolveDescriptor';
const FIXTURES = path.join(__dirname, '../../../../../../tests/utils/qti/fixtures');
@@ -28,6 +29,18 @@ const rebuild = item =>
hints: item.hints,
});
+describe('converted items the editor opens', () => {
+ it.each(
+ fs
+ .readdirSync(FIXTURES)
+ .filter(file => file.endsWith('.xml'))
+ .map(file => path.basename(file, '.xml')),
+ )('%s', name => {
+ const { interactions, itemBodyXml } = parseItem(read(name));
+ expect(isSupportedItem(interactions, itemBodyXml)).toBe(true);
+ });
+});
+
describe('a converted single-selection item', () => {
const original = read('single_selection');
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js
index 3bb4333cb6..fcedc997ae 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js
@@ -107,6 +107,33 @@ function adoptNode(node, doc = xmlDoc, plainNamespace = XHTML_NS) {
return el;
}
+const XMLNS_NS = 'http://www.w3.org/2000/xmlns/';
+
+/**
+ * Whether an element has attributes beyond namespace declarations.
+ *
+ * @param {Element} el
+ * @returns {boolean}
+ */
+export function hasNonNamespaceAttributes(el) {
+ return [...el.attributes].some(attr => attr.namespaceURI !== XMLNS_NS);
+}
+
+/**
+ * Whether a node is content: an element or text (CDATA included) beyond whitespace. Comments
+ * and the like carry none.
+ *
+ * @param {Node} node
+ * @returns {boolean}
+ */
+export function isContentNode(node) {
+ return (
+ node.nodeType === Node.ELEMENT_NODE ||
+ ((node.nodeType === Node.TEXT_NODE || node.nodeType === Node.CDATA_SECTION_NODE) &&
+ /[^ \t\r\n]/.test(node.nodeValue))
+ );
+}
+
/**
* Serialize XML nodes as an HTML string, for state that a rich text editor parses as HTML.
*
diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js
index ccf4bfc825..4096e3684e 100644
--- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js
+++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js
@@ -241,6 +241,32 @@ export const FREE_RESPONSE_ITEM_DOCUMENT = `
`;
+const TEXT_ENTRY_PARAGRAPH = '
';
+
+/** A block interaction with a stimulus beside it, which the editor would delete. */
+export const CHOICE_ITEM_DOCUMENT_WITH_STIMULUS = VALID_CHOICE_ITEM_DOCUMENT.replace(
+ '',
+ 'Read the passage.
',
+);
+
+/** An item the editor cannot reproduce, which would break its rules if they applied. */
+export const CHOICE_ITEM_DOCUMENT_NO_CORRECT_ANSWER_WITH_STIMULUS =
+ CHOICE_ITEM_DOCUMENT_NO_CORRECT_ANSWER.replace(
+ '',
+ 'Read the passage.
',
+ );
+
+export const TEXT_ENTRY_ITEM_DOCUMENT_SHARED_PARAGRAPH = FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ TEXT_ENTRY_PARAGRAPH,
+ 'Answer: km
',
+);
+
+/** Content after the interaction's ``, which the editor would move before it. */
+export const TEXT_ENTRY_ITEM_DOCUMENT_TRAILING_CONTENT = FREE_RESPONSE_ITEM_DOCUMENT.replace(
+ '
',
+ 'Show your working.
',
+);
+
export const NO_INTERACTION_ITEM_DOCUMENT = `
error).map(({ error }) => ({ code: error }));
}