Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ class HtmlParser private constructor(
context.assets,
htmlContentTextView.lineHeight.toFloat(),
cacheLatexRendering,
context as? Application ?: context.applicationContext as Application
context as? Application ?: context.applicationContext as Application,
equationColor = htmlContentTextView.currentTextColor
)
if (supportsConceptCards) {
handlersMap[CUSTOM_CONCEPT_CARD_TAG] = conceptCardTagHandler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class MathTagHandler(
private val assetManager: AssetManager,
private val lineHeight: Float,
private val cacheLatexRendering: Boolean,
private val application: Application
private val application: Application,
private val equationColor: Int? = null
) : CustomHtmlContentHandler.CustomTagHandler, CustomHtmlContentHandler.ContentDescriptionProvider {
override fun handleTag(
attributes: Attributes,
Expand All @@ -44,17 +45,21 @@ class MathTagHandler(
val content = MathContent.parseMathContent(
attributes.getJsonObjectValue(CUSTOM_MATH_MATH_CONTENT_ATTRIBUTE)
)
val useInlineRendering = when (attributes.getValue(CUSTOM_MATH_RENDER_TYPE_ATTRIBUTE)) {
var useInlineRendering = when (attributes.getValue(CUSTOM_MATH_RENDER_TYPE_ATTRIBUTE)) {
"inline" -> true
"block" -> false
else -> true
}
checkNotNull(imageRetriever) { "Expected imageRetriever to be not null." }
val equationColor = ResourcesCompat.getColor(
val resolvedEquationColor = equationColor ?: ResourcesCompat.getColor(
application.resources,
R.color.component_color_shared_equation_color,
null
)
// \frac renders with a horizontal bar only in block/display mode; force it so the bar is visible.
if (content is MathContent.MathAsLatex && content.rawLatex.contains("\\frac")) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am not fully sure about forcing every \frac expression into block mode here. This seems to override render-type="inline" for all fractions across the app. From checking the KotliTeX fraction code, \frac already creates a fraction line, so I am not sure the horizontal bar only exists in block/display mode.

Could you please share a small before/after screenshot showing that inline \frac loses the horizontal bar? This feels separate from the dark-mode contrast issue in #5809, and it may change inline lesson layout in other places.

Also, the old inline \frac test was changed to x^2, so we no longer have coverage for the explicit inline fraction case. Can we avoid this override, or add proof/tests that this behavior is intended? PTAL.

useInlineRendering = false
}
val newSpan = when (content) {
is MathContent.MathAsSvg -> {
ImageSpan(
Expand All @@ -71,7 +76,7 @@ class MathTagHandler(
imageRetriever.loadMathDrawable(
content.rawLatex,
lineHeight,
equationColor,
resolvedEquationColor,
type = if (useInlineRendering) INLINE_TEXT_IMAGE else BLOCK_IMAGE
),
useInlineRendering
Expand All @@ -82,7 +87,7 @@ class MathTagHandler(
lineHeight,
assetManager,
isMathMode = !useInlineRendering,
equationColor
resolvedEquationColor
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ class HtmlParserTest {
val textView: TextView = it.findViewById(R.id.test_html_content_text_view)
val htmlResult: Spannable = htmlParser.parseOppiaHtml(
"<oppia-noninteractive-math render-type=\"inline\" math_content-with-value=\"{" +
"&amp;quot;raw_latex&amp;quot;:&amp;quot;\\\\frac{2}{5}&amp;quot;}\">" +
"&amp;quot;raw_latex&amp;quot;:&amp;quot;x^2&amp;quot;}\">" +
"</oppia-noninteractive-math>",
textView,
supportsLinks = true,
Expand All @@ -1041,7 +1041,7 @@ class HtmlParserTest {
// The rendering mode should be inline for this render type.
val loadedInlineImages = testGlideImageLoader.getLoadedMathDrawables()
assertThat(loadedInlineImages).hasSize(1)
assertThat(loadedInlineImages.first().rawLatex).isEqualTo("\\frac{2}{5}")
assertThat(loadedInlineImages.first().rawLatex).isEqualTo("x^2")
assertThat(loadedInlineImages.first().useInlineRendering).isTrue()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ private const val MATH_WITHOUT_FILENAME_MARKUP =
"<oppia-noninteractive-math math_content-with-value=\"{&amp;quot;raw_latex&amp;quot;" +
":&amp;quot;\\\\frac{2}{5}&amp;quot;}\"></oppia-noninteractive-math>"

// A non-\frac expression so these tests exercise the inline code-path (not block-forced by \frac).
private const val MATH_WITHOUT_FILENAME_INLINE_MARKUP =
"<oppia-noninteractive-math math_content-with-value=\"{&amp;quot;raw_latex&amp;quot;" +
":&amp;quot;x^2&amp;quot;}\"></oppia-noninteractive-math>"

private const val MATH_WITHOUT_FILENAME_RENDER_TYPE_INLINE_MARKUP =
"<oppia-noninteractive-math render-type=\"inline\"" +
" math_content-with-value=\"{&amp;quot;raw_latex&amp;quot;" +
Expand Down Expand Up @@ -117,7 +122,7 @@ class MathTagHandlerTest {
fun testParseHtml_withMathMarkup_cachingOn_imageSpanHasCorrectMetrics() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
html = MATH_WITHOUT_FILENAME_INLINE_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)
Expand All @@ -142,7 +147,7 @@ class MathTagHandlerTest {
fun testParseHtml_withMathMarkup_cachingOn_drawsAtCorrectVerticalPosition() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
html = MATH_WITHOUT_FILENAME_INLINE_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)
Expand Down Expand Up @@ -306,43 +311,43 @@ class MathTagHandlerTest {
}

@Test
fun testParseHtml_withMathMarkup_missingFilename_includesCachedInlineLatexImageSpan() {
fun testParseHtml_withMathMarkup_missingFilename_withFrac_includesCachedBlockLatexImageSpan() {
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)

// The image span is a cached bitmap loaded from LaTeX.
// \frac expressions are always rendered in block mode for the horizontal fraction bar.
val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)
verify(mockImageRetriever)!!.loadMathDrawable(
capture(stringCaptor), capture(floatCaptor),
capture(colorCaptor), capture(retrieverTypeCaptor)
)
assertThat(stringCaptor.value).isEqualTo("\\frac{2}{5}")
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.INLINE_TEXT_IMAGE)
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.BLOCK_IMAGE)
}

@Test
fun testParseHtml_withMathMarkup_missingFilename_inlineMode_includesCachedInlineLatexImageSpan() {
fun testParseHtml_withMathMarkup_missingFilename_inlineMode_includesCachedBlockLatexImageSpan() {
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_RENDER_TYPE_INLINE_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)

// The image span is a cached bitmap loaded from LaTeX.
// \frac overrides even an explicit inline render-type to ensure the horizontal bar is visible.
val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)
verify(mockImageRetriever)!!.loadMathDrawable(
capture(stringCaptor), capture(floatCaptor),
capture(colorCaptor), capture(retrieverTypeCaptor)
)
assertThat(stringCaptor.value).isEqualTo("\\frac{2}{5}")
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.INLINE_TEXT_IMAGE)
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.BLOCK_IMAGE)
}

@Test
Expand Down Expand Up @@ -460,7 +465,7 @@ class MathTagHandlerTest {
capture(colorCaptor), capture(retrieverTypeCaptor)
)
assertThat(stringCaptor.value).isEqualTo("\\frac{2}{5}")
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.INLINE_TEXT_IMAGE)
assertThat(retrieverTypeCaptor.value).isEqualTo(ImageRetriever.Type.BLOCK_IMAGE)
}

@Test
Expand All @@ -481,7 +486,7 @@ class MathTagHandlerTest {
.containsExactly("\\frac{3}{8}", "\\frac{2}{5}")
.inOrder()
assertThat(retrieverTypeCaptor.allValues)
.containsExactly(ImageRetriever.Type.INLINE_TEXT_IMAGE, ImageRetriever.Type.INLINE_TEXT_IMAGE)
.containsExactly(ImageRetriever.Type.BLOCK_IMAGE, ImageRetriever.Type.BLOCK_IMAGE)
.inOrder()
}

Expand Down Expand Up @@ -531,14 +536,58 @@ class MathTagHandlerTest {
assertThat(parsedHtmlStr).contains(" and ")
}

private fun createMathTagHandler(cacheLatexRendering: Boolean): MathTagHandler {
@Test
fun testParseHtml_withMathMarkup_cachingOff_withProvidedEquationColor_usesProvidedColor() {
val customColor = Color.MAGENTA
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = mapOf(
CUSTOM_MATH_TAG to createMathTagHandler(
cacheLatexRendering = false,
equationColor = customColor
)
)
)

val mathExpressionSpans = parsedHtml.getSpansFromWholeString(MathExpressionSpan::class)
assertThat(mathExpressionSpans[0].equationColor).isEqualTo(customColor)
}

@Test
fun testParseHtml_withMathMarkup_cachingOn_withProvidedEquationColor_usesProvidedColor() {
val customColor = Color.CYAN
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_INLINE_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = mapOf(
CUSTOM_MATH_TAG to createMathTagHandler(
cacheLatexRendering = true,
equationColor = customColor
)
)
)

verify(mockImageRetriever)!!.loadMathDrawable(
capture(stringCaptor), capture(floatCaptor),
capture(colorCaptor), capture(retrieverTypeCaptor)
)
assertThat(colorCaptor.value).isEqualTo(customColor)
}
Comment on lines +559 to +577

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done — switched the fixture to MATH_WITHOUT_FILENAME_INLINE_MARKUP (uses x^2, no \frac) so the color assertion is independent of the block-forcing behavior, and removed the BLOCK_IMAGE assertion.


private fun createMathTagHandler(
cacheLatexRendering: Boolean,
equationColor: Int? = null
): MathTagHandler {
// Pick an arbitrary line height since rendering doesn't actually happen in tests.
return MathTagHandler(
consoleLogger,
context.assets,
lineHeight = 10.0f,
cacheLatexRendering,
application = context.applicationContext as Application
application = context.applicationContext as Application,
equationColor = equationColor
)
}

Expand Down
Loading