Repository: oppia/oppia
File: core/domain/voiceover_regeneration_services.py (Python)

Code before:
        escaped_math_content = element.get('math_content-with-value')
        math_content = json.loads(html.unescape(escaped_math_content))
        latex_expr = math_content['raw_latex']
        converter = latex2text.LatexNodes2Text()
        element.string = converter.latex_to_text(latex_expr)
        element.name = 'p'
    return element


def parse_html(html_content: str) -> str:
    """The method processes the HTML content and extracts the plain text.

    Args:
        html_content: str. HTML content that will be transformed into plain
            text.

    Returns:
        str. The plain text retrieved from the HTML content.

    Raises:
        Exception. The HTML content contains invalid or unsupported tags.
    """
    soup = bs4.BeautifulSoup(html_content, 'html.parser')
    all_tags = {tag.name for tag in soup.find_all()}

    unknown_tags = (
        all_tags -
        set(ALLOWED_CUSTOM_OPPIA_RTE_TAGS) -
        set(ALLOWED_GENERAL_OPPIA_RTE_TAGS)
    )

    if unknown_tags:
        raise Exception(
            'HTML content contains invalid or unsupported tags: %s' % (
                ', '.join(unknown_tags)))

    for custom_tag_element in ALLOWED_CUSTOM_OPPIA_RTE_TAGS:
        for element in soup.find_all(custom_tag_element):
            convert_custom_oppia_tags_to_generic_tags(element)

    text_content: str = get_text_with_delimiters(
        soup, delimiter=feconf.OPPIA_CONTENT_TAG_DELIMITER)

    return text_content


def get_text_with_delimiters(soup: bs4.BeautifulSoup, delimiter: str) -> str:
    """The method extracts text from the BeautifulSoup object and
    adds delimiters between text segments based on the block-level HTML tags.

    Args:

Review comment (suggestion):
I think it's fine to have an approach where we allowlist specific tags. However, we can't be defining separate lists of "allowed" RTE tags in different places (like in this file); instead, there should be one single reference. For example, I think there is already a reference elsewhere stating which RTE components a particular RTE supports, and any sort of custom-tag validation we do here should refer to that.

So there are two approaches:

- Voiceover regeneration handles all tags, and translates some of them to empty strings. There is a backend test that verifies that the voiceover regeneration actually has a translation rule defined for all known RTE component tags (there should be an existing constant for this somewhere).

- Voiceover regeneration handles only tags that can be entered in the RTEs that support voiceover-ing. In that case the RTE configs that support voiceovers should be clearly labelled as such, with an error thrown if generation of voiceovers is attempted for a config that doesn't explicitly support voiceovers. Similarly to the above, there should be a backend test that takes all custom RTE components defined in the voiceover-enabled configs, finds their union, and confirms that each of them have a "translation rule" as defined above.

Either of the above is fine, but a custom approach which involves adding new lists of components isn't, because it doesn't solve the issue of ensuring that if a new component is added, the developer is forced to do the right thing with regards to voiceovers. On the other hand, the above approaches do force that -- the first forces writing the rule, and in the second case, any issues will become obvious when the new component is used in the RTE (since, if the RTE is voiceoverable, the component will be auto-included in the list of things that need to have a voiceover rule).

Please let me know if you have any further questions about this -- thanks.

Code after:
    return output_str


def _return_empty_string(_: bs4.Tag) -> str:
    """Returns an empty string for the given element.

    Args:
        _: Tag. The HTML tag for which to return an empty string.

    Returns:
        str. An empty string.
    """
    return ''


# A dictionary mapping custom Oppia RTE tags to their respective text extraction
# methods used during voiceover regeneration. Tags like images, videos, etc
# currently return an empty string, indicating that they are not yet included in
# voiceover generation (but may be in the future). When introducing new tags,
# add a corresponding extraction rule here to ensure their learner-facing text
# can be included in voiceovers.
CUSTOM_RTE_TAGS_TO_VOICEOVER_TEXT_EXTRACTION_RULES = {
    'oppia-noninteractive-link': _extract_text_from_link_tag,
    'oppia-noninteractive-math': _extract_text_from_math_tag,
    'oppia-noninteractive-skillreview': _extract_text_from_skillreview_tag,
    'oppia-noninteractive-collapsible': _return_empty_string,
    'oppia-noninteractive-image': _return_empty_string,
    'oppia-noninteractive-video': _return_empty_string,
    'oppia-noninteractive-tabs': _return_empty_string,
    'oppia-noninteractive-workedexample': _return_empty_string
}


def convert_custom_oppia_tags_to_generic_tags(element: bs4.Tag) -> bs4.Tag:
    """The method transforms custom Oppia tags into standard paragraph tags.

    Args:
        element: Tag. A custom Oppia tag that will be transformed into a
            standard paragraph tag.

    Returns:
        Tag. The transformed paragraph tag.
    """
    tag_name = element.name
    voiceover_text_extractor_fn = (
        CUSTOM_RTE_TAGS_TO_VOICEOVER_TEXT_EXTRACTION_RULES.get(
            tag_name, _return_empty_string))

    element.string = voiceover_text_extractor_fn(element)
    element.name = 'p'
    return element