Skip to content

Commit

Permalink
Fix #4848: Ensure Complete Content Descriptions for Custom HTML Tags …
Browse files Browse the repository at this point in the history
…in CustomHtmlContentHandler (#5614)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fix #4848: This PR resolves the issue with incomplete content
descriptions being generated for custom HTML tags. The existing
implementation stripped out spans and custom tag content, leading to
incomplete descriptions.

The updated implementation introduces a custom generation pipeline for
content descriptions in `CustomHtmlContentHandler`. It ensures that all
custom tag content is correctly processed and appended, resulting in
complete and accurate descriptions.

### Key Changes:
- Added a `StringBuilder` to accumulate content descriptions and a
`MutableMap` to handle custom tag descriptions.
- Implemented `getContentDescription()` to combine default text with
custom tag content descriptions, ensuring seamless integration.
- Introduced a `ContentDescriptionProvider` interface for handling
custom tags.
- Updated relevant tag handlers (`ImageTagHandler`,
`ConceptCardTagHandler`, `LiTagHandler`, `PolicyPageTagHandler` and
`MathTagHandler`) to implement the new interface.
- Verified the implementation with appropriate test cases.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
manas-yu and adhiamboperes authored Jan 14, 2025
1 parent 11b1d89 commit 4f5aec6
Show file tree
Hide file tree
Showing 12 changed files with 337 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import org.xml.sax.Attributes

/** The custom tag corresponding to [ConceptCardTagHandler]. */
const val CUSTOM_CONCEPT_CARD_TAG = "oppia-noninteractive-skillreview"
const val CUSTOM_CONCEPT_CARD_SKILL_ID = "skill_id-with-value"
const val CUSTOM_CONCEPT_CARD_TEXT_VALUE = "text-with-value"

// https://mohammedlakkadshaw.com/blog/handling-custom-tags-in-android-using-html-taghandler.html/
class ConceptCardTagHandler(
private val listener: ConceptCardLinkClickListener,
private val consoleLogger: ConsoleLogger
) : CustomHtmlContentHandler.CustomTagHandler {
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
override fun handleTag(
attributes: Attributes,
openIndex: Int,
Expand All @@ -24,8 +26,8 @@ class ConceptCardTagHandler(
imageRetriever: CustomHtmlContentHandler.ImageRetriever?
) {
// Replace the custom tag with a clickable piece of text based on the tag's customizations.
val skillId = attributes.getJsonStringValue("skill_id-with-value")
val text = attributes.getJsonStringValue("text-with-value")
val skillId = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_SKILL_ID)
val text = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_TEXT_VALUE)
if (skillId != null && text != null) {
val spannableBuilder = SpannableStringBuilder(text)
spannableBuilder.setSpan(
Expand All @@ -48,4 +50,12 @@ class ConceptCardTagHandler(
*/
fun onConceptCardLinkClicked(view: View, skillId: String)
}

override fun getContentDescription(attributes: Attributes): String? {
val skillId = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_SKILL_ID)
val text = attributes.getJsonStringValue(CUSTOM_CONCEPT_CARD_TEXT_VALUE)
return if (!skillId.isNullOrBlank() && !text.isNullOrBlank()) {
"$text concept card $skillId"
} else ""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,17 @@ class CustomHtmlContentHandler private constructor(
private var originalContentHandler: ContentHandler? = null
private var currentTrackedTag: TrackedTag? = null
private val currentTrackedCustomTags = ArrayDeque<TrackedCustomTag>()
private val contentDescriptionBuilder = StringBuilder()
private val tagContentDescriptions = mutableMapOf<Int, String>()
private var isInListItem = false
private var pendingNewline = false
private val blockTags = setOf("p", "ol", "ul", "li", "oppia-ul", "oppia-ol", "oppia-li", "div")

override fun endElement(uri: String?, localName: String?, qName: String?) {
originalContentHandler?.endElement(uri, localName, qName)
if (localName in blockTags) {
isInListItem = false
}
currentTrackedTag = null
}

Expand All @@ -45,6 +53,11 @@ class CustomHtmlContentHandler private constructor(

override fun characters(ch: CharArray?, start: Int, length: Int) {
originalContentHandler?.characters(ch, start, length)
if (pendingNewline) {
contentDescriptionBuilder.append('\n')
pendingNewline = false
}
ch?.let { contentDescriptionBuilder.append(String(it, start, length)) }
}

override fun endDocument() {
Expand All @@ -56,6 +69,10 @@ class CustomHtmlContentHandler private constructor(
// Defer custom tag management to the tag handler so that Android's element parsing takes
// precedence.
currentTrackedTag = TrackedTag(checkNotNull(localName), checkNotNull(atts))
if (localName in blockTags) {
pendingNewline = true
isInListItem = true
}
originalContentHandler?.startElement(uri, localName, qName, atts)
}

Expand Down Expand Up @@ -110,6 +127,14 @@ class CustomHtmlContentHandler private constructor(
"Expected tracked tag $currentTrackedTag to match custom tag: $tag"
}
val (_, attributes, openTagIndex) = currentTrackedCustomTag

val handler = customTagHandlers.getValue(tag)
if (handler is ContentDescriptionProvider) {
val contentDesc = handler.getContentDescription(attributes)
if (contentDesc != null) {
tagContentDescriptions[openTagIndex] = contentDesc
}
}
customTagHandlers.getValue(tag).handleClosingTag(output, indentation = 0, tag)
customTagHandlers.getValue(tag)
.handleTag(attributes, openTagIndex, output.length, output, imageRetriever)
Expand All @@ -123,7 +148,26 @@ class CustomHtmlContentHandler private constructor(
val attributes: Attributes,
val openTagIndex: Int
)

/**
* Returns the complete content description for the processed HTML, including descriptions
* from all custom tags.
*/
private fun getContentDescription(): String {
val rawDesc = buildString {
var lastIndex = 0
tagContentDescriptions.entries.sortedBy { it.key }.forEach { (index, description) ->
if (index > lastIndex) {
append(contentDescriptionBuilder.substring(lastIndex, index))
}
append(description)
lastIndex = index
}
if (lastIndex < contentDescriptionBuilder.length) {
append(contentDescriptionBuilder.substring(lastIndex))
}
}
return rawDesc.replace(Regex("\n+"), "\n").trim()
}
/** Handler interface for a custom tag and its attributes. */
interface CustomTagHandler {
/**
Expand Down Expand Up @@ -166,6 +210,15 @@ class CustomHtmlContentHandler private constructor(
fun handleClosingTag(output: Editable, indentation: Int, tag: String) {}
}

/** Handler Interface for tag handlers that provide content descriptions. */
interface ContentDescriptionProvider {
/**
* Returns a content description string for this tag based on its attributes,
* or null if no description is available.
*/
fun getContentDescription(attributes: Attributes): String?
}

/**
* Retriever of images for custom tag handlers. The built-in Android analog for this class is
* Html's ImageGetter.
Expand Down Expand Up @@ -196,6 +249,24 @@ class CustomHtmlContentHandler private constructor(
}

companion object {
/**
* Returns the content description for the HTML content, processing all custom tags that implement
* [ContentDescriptionProvider].
*/
fun <T> getContentDescription(
html: String,
imageRetriever: T?,
customTagHandlers: Map<String, CustomTagHandler>
): String where T : Html.ImageGetter, T : ImageRetriever {
val handler = CustomHtmlContentHandler(customTagHandlers, imageRetriever)
HtmlCompat.fromHtml(
"<init-custom-handler/>$html",
HtmlCompat.FROM_HTML_MODE_LEGACY,
imageRetriever,
handler
)
return handler.getContentDescription()
}
/**
* Returns a new [Spannable] with HTML parsed from [html] using the specified [imageRetriever]
* for handling image retrieval, and map of tags to [CustomTagHandler]s for handling custom
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private const val CUSTOM_IMG_CAPTION_ATTRIBUTE = "caption-with-value"
*/
class ImageTagHandler(
private val consoleLogger: ConsoleLogger
) : CustomHtmlContentHandler.CustomTagHandler {
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
override fun handleTag(
attributes: Attributes,
openIndex: Int,
Expand Down Expand Up @@ -101,4 +101,11 @@ class ImageTagHandler(
"Failed to parse $CUSTOM_IMG_CAPTION_ATTRIBUTE"
)
}

override fun getContentDescription(attributes: Attributes): String {
val altValue = attributes.getJsonStringValue(CUSTOM_IMG_ALT_TEXT_ATTRIBUTE)
return if (!altValue.isNullOrBlank()) {
"Image illustrating $altValue"
} else ""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.text.Editable
import android.text.Spannable
import android.text.Spanned
import org.oppia.android.util.locale.OppiaLocale
import org.xml.sax.Attributes
import java.util.Stack

/** The custom <li> tag corresponding to [LiTagHandler]. */
Expand All @@ -23,7 +24,7 @@ const val CUSTOM_LIST_OL_TAG = "oppia-ol"
class LiTagHandler(
private val context: Context,
private val displayLocale: OppiaLocale.DisplayLocale
) : CustomHtmlContentHandler.CustomTagHandler {
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
private val pendingLists = Stack<ListTag<*, *>>()
private val latestPendingList: ListTag<*, *>?
get() = pendingLists.lastOrNull()
Expand Down Expand Up @@ -291,4 +292,8 @@ class LiTagHandler(
private fun <T : Mark<*>> Spannable.addMark(mark: T) =
setSpan(mark, length, length, Spanned.SPAN_MARK_MARK)
}

override fun getContentDescription(attributes: Attributes): String {
return ""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class MathTagHandler(
private val lineHeight: Float,
private val cacheLatexRendering: Boolean,
private val application: Application
) : CustomHtmlContentHandler.CustomTagHandler {
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
override fun handleTag(
attributes: Attributes,
openIndex: Int,
Expand Down Expand Up @@ -138,4 +138,9 @@ class MathTagHandler(
}
}
}

override fun getContentDescription(attributes: Attributes): String {
val mathVal = attributes.getJsonObjectValue(CUSTOM_MATH_MATH_CONTENT_ATTRIBUTE)
return mathVal?.let { "Math content $it" } ?: ""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private const val TERMS_OF_SERVICE = "Terms of Service"
class PolicyPageTagHandler(
private val listener: PolicyPageLinkClickListener,
private val consoleLogger: ConsoleLogger
) : CustomHtmlContentHandler.CustomTagHandler {
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
override fun handleTag(
attributes: Attributes,
openIndex: Int,
Expand Down Expand Up @@ -83,4 +83,11 @@ class PolicyPageTagHandler(
*/
fun onPolicyPageLinkClicked(policyType: PolicyType)
}

override fun getContentDescription(attributes: Attributes): String {
return when (attributes.getJsonStringValue("link")) {
TERMS_OF_SERVICE_PAGE, PRIVACY_POLICY_PAGE -> "Link to "
else -> ""
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ class ConceptCardTagHandlerTest {
assertThat(clickableSpans).hasLength(1)
}

@Test
fun testGetContentDescription_withConceptCardTag() {
val contentDescription =
CustomHtmlContentHandler.getContentDescription(
html = CONCEPT_CARD_LINK_MARKUP_1,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithConceptCardSupport
)
assertThat(contentDescription).isEqualTo("refresher lesson concept card skill_id_1")
}

@Test
fun testParseHtml_withConceptCardMarkup_addsLinkText() {
val parsedHtml =
Expand Down
Loading

0 comments on commit 4f5aec6

Please sign in to comment.