Fix #6430, #6431: Preserve worked example list spacing

utility/src/main/java/org/oppia/android/util/parser/html/HtmlParser.kt

@@ -220,20 +220,21 @@ class HtmlParser private constructor(
    * renders ordered lists as bullets rather than as numbers).
    */
   private fun replaceListTags(html: String): String {
-    var adjustedHtml = html
-    if ("<li>" in adjustedHtml) {
-      adjustedHtml = adjustedHtml.replace("<li>", "<$CUSTOM_LIST_LI_TAG>")
-        .replace("</li>", "</$CUSTOM_LIST_LI_TAG>")
-    }
-    if ("<ul>" in adjustedHtml) {
-      adjustedHtml = adjustedHtml.replace("<ul>", "<$CUSTOM_LIST_UL_TAG>")
-        .replace("</ul>", "</$CUSTOM_LIST_UL_TAG>")
-    }
-    if ("<ol>" in adjustedHtml) {
-      adjustedHtml = adjustedHtml.replace("<ol>", "<$CUSTOM_LIST_OL_TAG>")
-        .replace("</ol>", "</$CUSTOM_LIST_OL_TAG>")
-    }
-    return adjustedHtml
+    return html
+      .replaceListTag(originalTag = "li", replacementTag = CUSTOM_LIST_LI_TAG)
+      .replaceListTag(originalTag = "ul", replacementTag = CUSTOM_LIST_UL_TAG)
+      .replaceListTag(originalTag = "ol", replacementTag = CUSTOM_LIST_OL_TAG)
+  }
+
+  /**
+   * Replaces opening and closing [originalTag] tags while preserving attributes such as the XHTML
+   * namespace included by Oppia's rich-text editor.
+   */
+  private fun String.replaceListTag(originalTag: String, replacementTag: String): String {
+    return replace(
+      Regex("""<(/?)$originalTag(?=[\s>])""", RegexOption.IGNORE_CASE),
+      "<$1$replacementTag"
+    )
   }
 
   private fun trimSpannable(spannable: SpannableStringBuilder): SpannableStringBuilder {

utility/src/main/java/org/oppia/android/util/parser/html/WorkedExampleTagHandler.kt

@@ -59,6 +59,14 @@ class WorkedExampleTagHandler(
       return
     }
 
+    // Inserting a worked example at the end of a list-item span causes SpannableStringBuilder to
+    // extend that span over the inserted content. This happens when consecutive worked examples
+    // have answers that end in lists, and makes every following example accumulate another list
+    // margin. Remember those spans so their original boundary can be restored after replacement.
+    val precedingListItemSpans = output
+      .getSpans(0, openIndex, ListItemLeadingMarginSpan::class.java)
+      .filter { output.getSpanEnd(it) == openIndex }
+
     val parsedWorkedExample = SpannableStringBuilder().apply {
       // Worked examples are block content, so they're separated from whatever precedes them by a
       // blank line. Existing line breaks count towards that blank line, so an example that follows
@@ -95,6 +103,14 @@ class WorkedExampleTagHandler(
       Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
     )
     output.replace(openIndex, closeIndex, parsedWorkedExample)
+    precedingListItemSpans.forEach { span ->
+      output.setSpan(
+        span,
+        output.getSpanStart(span),
+        openIndex,
+        output.getSpanFlags(span)
+      )
+    }
   }
 
   // Note that this is implemented in addition to getContentDescription since the two are used in

utility/src/test/java/org/oppia/android/util/parser/html/HtmlParserTest.kt

@@ -10,6 +10,7 @@ import android.text.style.BulletSpan
 import android.text.style.ClickableSpan
 import android.text.style.ImageSpan
 import android.text.style.LeadingMarginSpan
+import android.text.style.URLSpan
 import android.view.View
 import android.widget.TextView
 import androidx.appcompat.app.AppCompatActivity
@@ -30,6 +31,7 @@ import com.google.common.truth.Truth.assertThat
 import dagger.Component
 import org.hamcrest.CoreMatchers
 import org.hamcrest.Matchers.not
+import org.json.JSONObject
 import org.junit.After
 import org.junit.Before
 import org.junit.Rule
@@ -984,6 +986,125 @@ class HtmlParserTest {
     )
   }
 
+  @Test
+  fun testHtmlContent_withConsecutiveWorkedExamplesEndingInLists_doesNotAccumulateMargins() {
+    val htmlParser = htmlParserFactory.create(
+      resourceBucketName,
+      entityType = "",
+      entityId = "",
+      imageCenterAlign = false,
+      displayLocale = appLanguageLocaleHandler.getDisplayLocale()
+    )
+    val textView = TextView(context)
+    val firstExample = createWorkedExampleMarkup(
+      questionHtml = "First question",
+      answerHtml =
+        "<ul xmlns=\"http://www.w3.org/1999/xhtml\"><li>First answer</li></ul>"
+    )
+    val secondExample = createWorkedExampleMarkup(
+      questionHtml = "Second question",
+      answerHtml =
+        "<ul xmlns=\"http://www.w3.org/1999/xhtml\"><li>Second answer</li></ul>"
+    )
+    val thirdExample = createWorkedExampleMarkup(
+      questionHtml = "Third question",
+      answerHtml =
+        "<ul xmlns=\"http://www.w3.org/1999/xhtml\"><li>Third answer</li></ul>"
+    )
+
+    val htmlResult = htmlParser.parseOppiaHtml(
+      firstExample + secondExample + thirdExample,
+      textView,
+      workedExampleLabels = WORKED_EXAMPLE_LABELS
+    )
+
+    val questionRanges =
+      listOf("First question", "Second question", "Third question").map { question ->
+        val questionIndex = htmlResult.toString().indexOf(question)
+        questionIndex until questionIndex + question.length
+      }
+    assertThat(
+      questionRanges.map { range ->
+        htmlResult.getSpans(
+          range.first,
+          range.last + 1,
+          LeadingMarginSpan.Standard::class.java
+        ).size
+      }
+    ).containsExactly(1, 1, 1).inOrder()
+    assertThat(
+      questionRanges.map { range ->
+        htmlResult.getSpans(
+          range.first,
+          range.last + 1,
+          ListItemLeadingMarginSpan::class.java
+        ).size
+      }
+    ).containsExactly(0, 0, 0).inOrder()
+  }
+
+  @Test
+  fun testHtmlContent_withWorkedExampleNamespacedList_preservesBlockSpacing() {
+    val htmlParser = htmlParserFactory.create(
+      resourceBucketName,
+      entityType = "",
+      entityId = "",
+      imageCenterAlign = false,
+      displayLocale = appLanguageLocaleHandler.getDisplayLocale()
+    )
+    val textView = TextView(context)
+    val workedExampleMarkup = createWorkedExampleMarkup(
+      questionHtml = "Where should the content be separated?",
+      answerHtml =
+        "<ol xmlns=\"http://www.w3.org/1999/xhtml\"><li>Outer item one:<ul>" +
+          "<li>Inner item one.</li><li>Inner item two.</li></ul></li>" +
+          "<li>Outer item two.</li></ol><p>Following paragraph.</p>"
+    )
+
+    val htmlResult = htmlParser.parseOppiaHtml(
+      workedExampleMarkup,
+      textView,
+      workedExampleLabels = WORKED_EXAMPLE_LABELS
+    )
+
+    assertThat(htmlResult.getSpansFromWholeString(ListItemLeadingMarginSpan.OlSpan::class))
+      .hasLength(2)
+    assertThat(htmlResult.getSpansFromWholeString(ListItemLeadingMarginSpan.UlSpan::class))
+      .hasLength(2)
+    assertThat(htmlResult.toString()).contains("Inner item two.\nOuter item two.")
+    assertThat(htmlResult.toString()).contains("Outer item two.\n\nFollowing paragraph.")
+  }
+
+  @Test
+  fun testHtmlContent_withWorkedExampleNamespacedList_doesNotCreateFalseLinks() {
+    val htmlParser = htmlParserFactory.create(
+      resourceBucketName,
+      entityType = "",
+      entityId = "",
+      imageCenterAlign = false,
+      displayLocale = appLanguageLocaleHandler.getDisplayLocale()
+    )
+    val textView = TextView(context)
+    val workedExampleMarkup = createWorkedExampleMarkup(
+      questionHtml = "Add the decimals.",
+      answerHtml =
+        "<ul xmlns=\"http://www.w3.org/1999/xhtml\">" +
+          "<li>Add a decimal point before the tenths place.</li>" +
+          "<li>Now add the place value digits.</li>" +
+          "<li>The answer is 5.682.</li></ul>"
+    )
+
+    val htmlResult = htmlParser.parseOppiaHtml(
+      workedExampleMarkup,
+      textView,
+      workedExampleLabels = WORKED_EXAMPLE_LABELS
+    )
+
+    assertThat(htmlResult.toString()).contains("place.\nNow")
+    assertThat(htmlResult.toString()).contains("digits.\nThe")
+    assertThat(htmlResult.getSpansFromWholeString(URLSpan::class)).isEmpty()
+  }
+
   @Test
   fun testHtmlContent_withUrl_hasClickableSpanAndCorrectText() {
     val htmlParser = htmlParserFactory.create(
@@ -1430,6 +1551,21 @@ class HtmlParserTest {
     return DisplayLocaleImpl(context, formattingLocale, machineLocale, formatterFactory)
   }
 
+  private fun createWorkedExampleMarkup(questionHtml: String, answerHtml: String): String {
+    return "<$CUSTOM_WORKED_EXAMPLE_TAG " +
+      "question-with-value=\"${questionHtml.encodeAsWorkedExampleAttribute()}\" " +
+      "answer-with-value=\"${answerHtml.encodeAsWorkedExampleAttribute()}\">" +
+      "</$CUSTOM_WORKED_EXAMPLE_TAG>"
+  }
+
+  private fun String.encodeAsWorkedExampleAttribute(): String {
+    return JSONObject.quote(this)
+      .replace("&", "&amp;amp;")
+      .replace("\"", "&amp;quot;")
+      .replace("<", "&amp;lt;")
+      .replace(">", "&amp;gt;")
+  }
+
   private fun Spannable.getTextForSpan(span: Any): String =
     subSequence(getSpanStart(span), getSpanEnd(span)).toString()