Skip to content

fix: support AnnotatableBlock in the new content runtime#268

Open
farhan wants to merge 1 commit into
mainfrom
farhan/fix-annotatable-block-bug
Open

fix: support AnnotatableBlock in the new content runtime#268
farhan wants to merge 1 commit into
mainfrom
farhan/fix-annotatable-block-bug

Conversation

@farhan

@farhan farhan commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Closes: openedx/openedx-platform#38809

What

Add a parse_xml_new_runtime override to AnnotatableBlock so the block can be loaded by the new openedx_content-based content runtime (Content Libraries).

Why

While extracting the Annotatable block from openedx-platform, we parted ways with RawMixin, which contains the parse_xml_new_runtime method. That method is still required by the extracted block to be compatible with the new content runtime, and it was missed during extraction.

Without it, opening an annotatable block in a Content Library crashes with:

NotImplementedError: XML Serialization is only supported with OpenedXContentRuntime

The new runtime loads blocks via parse_xml_new_runtime. Annotatable only inherited the LegacyXmlMixin fallback, which delegates to the base XBlock.parse_xml. That base parser treats every child XML node as a child block and calls runtime.add_node_as_child(), which the new runtime does not support (it raises the error above). Since annotatable OLX always contains nested markup (<instructions>, <p>, <annotation>), loading always failed.

Fix

Copy the parse_xml_new_runtime method from RawMixin into AnnotatableBlock. It captures the full inner XML into the data field and strips the child nodes before delegating to the base parser.

Case study: Problem block (same pattern, already done)

This is exactly how the extracted Problem block was handled:

The same was done for the html and video blocks. Annotatable was the one raw-data block where this copy was omitted during extraction; this PR brings it in line.

Manual testing

  1. Create an Annotatable Block in the Course
  2. Copy the Block from the Course and paste into Library as Component. Or Import the course components into the Library.
  3. Before this change: 500 error (NotImplementedError in studio logs). After: the component renders.

Screenshot

Annotatable Block successfully loaded into the Content Library

screencapture-apps-local-openedx-io-2001-authoring-library-lib-axim-3-components-lb-axim-3-annotatable-annotation-eb2f56-2026-06-26-21_03_52

While extracting the Annotatable block from openedx-platform, we parted
ways with RawMixin, which contains the parse_xml_new_runtime method. That
method is still required by the extracted annotatable block to make it
compatible with the new content runtime, and it was missed during
extraction. So this change copies the method from RawMixin into the
extracted AnnotatableBlock.

Without this, loading an annotatable block in a Content Library crashed
with "XML Serialization is only supported with OpenedXContentRuntime",
because the base XBlock.parse_xml treats annotatable's nested OLX
(<instructions>, <p>, <annotation>) as child blocks and calls
runtime.add_node_as_child(), which the new runtime does not implement.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bradenmacdonald

Copy link
Copy Markdown

Usually it's better to just implement parse_xml, unless you really need the parsing to work differently in the new runtime vs the old, in which case you can implement both parse_xml_new_runtime and parse_xml with different behavior. But having one simple parse_xml function is better for consistency.

We only introduced parse_xml_new_runtime because some XBlocks like video were doing crazy things in their parse_xml method, and we couldn't change that to be sane without breaking existing stuff.

@farhan

farhan commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

But having one simple parse_xml function is better for consistency.

We only introduced parse_xml_new_runtime because some XBlocks like video were doing crazy things in their parse_xml method

@bradenmacdonald Thanks for the context — that's really helpful background on why parse_xml_new_runtime exists in the first place.

For the extraction work, our focus was largely a lift-and-shift of the existing logic/methods from edx-platform, so this PR follows that same principle and copies the method over to restore parity.

Current status in xblocks-core: Video, Problem, HTML, and Annotatable now all carry their own parse_xml_new_runtime definitions.

Good point that a single parse_xml is the better long-term shape, with *_new_runtime really only needed for blocks like Video. Since Problem, HTML, and Annotatable may not need the separate method, we can open a follow-up ticket to study consolidating them into one parse_xml — I think keeping this PR scoped to fix sounds reasonable

cc: @kdmccormick

@farhan farhan requested a review from irtazaakram June 29, 2026 08:53
block = super().parse_xml_new_runtime(node, runtime, keys)
except AttributeError:
block = super().parse_xml(node, runtime, keys)
block.data = data_field_value

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential crash when importing a bare <annotatable/> node.

definition_from_xml returns {'data': ''} when the node has no children and no attributes (the len == 0 early-exit branch). The assignment here then unconditionally overwrites whatever super().parse_xml set — including the non-empty field default — with an empty string. Any subsequent call to student_view hits etree.fromstring('') in _render_content and raises XMLSyntaxError: Document is empty.

A bare <annotatable/> is an unusual OLX shape, but the guard is cheap:

if data_field_value:
    block.data = data_field_value

This matches how RawMixin handles the equivalent case in edx-platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attempt to import "annotatable" block content into library fails

3 participants