Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Commit

Permalink
Fix errors in tests related to whitespaces and span ordering
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmartinesp
jmartinesp committed Jul 17, 2024
1 parent 219dc93 commit 5cfb891
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 40 deletions.
6 changes: 3 additions & 3 deletions .../java/io/element/android/wysiwyg/inputhandlers/InterceptInputConnectionIntegrationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ class InterceptInputConnectionIntegrationTest {
textView.text.dumpSpans().joinToString(",\n"), equalTo(
"""
hello: android.widget.TextView.ChangeWatcher (0-5) fl=#6553618,
hello: io.element.android.wysiwyg.view.spans.QuoteSpan (0-5) fl=#17,
hello: io.element.android.wysiwyg.view.spans.QuoteSpan (0-5) fl=#65553,
hello: android.text.style.UnderlineSpan (0-5) fl=#289,
hello: android.view.inputmethod.ComposingText (0-5) fl=#289,
hello: android.text.method.TextKeyListener (0-5) fl=#18,
Expand Down Expand Up @@ -408,8 +408,8 @@ class InterceptInputConnectionIntegrationTest {
textView.text.dumpSpans(), equalTo(
listOf(
"Test\n$NBSP: android.widget.TextView.ChangeWatcher (0-6) fl=#6553618",
"Test\n$NBSP: io.element.android.wysiwyg.view.spans.QuoteSpan (0-6) fl=#65553",
"$NBSP: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (5-6) fl=#17",
"Test\n$NBSP: io.element.android.wysiwyg.view.spans.QuoteSpan (0-6) fl=#17",
"Test\n$NBSP: android.text.method.TextKeyListener (0-6) fl=#18",
"Test\n$NBSP: android.widget.Editor.SpanController (0-6) fl=#18",
": android.text.Selection.START (5-5) fl=#546",
Expand All @@ -426,7 +426,7 @@ class InterceptInputConnectionIntegrationTest {
textView.text.dumpSpans(), equalTo(
listOf(
"Test\n$NBSP: android.widget.TextView.ChangeWatcher (0-6) fl=#6553618",
"Test: io.element.android.wysiwyg.view.spans.QuoteSpan (0-4) fl=#17",
"Test: io.element.android.wysiwyg.view.spans.QuoteSpan (0-4) fl=#65553",
"$NBSP: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (5-6) fl=#17",
"Test\n$NBSP: android.text.method.TextKeyListener (0-6) fl=#18",
"Test\n$NBSP: android.widget.Editor.SpanController (0-6) fl=#18",
Expand Down
108 changes: 95 additions & 13 deletions ...forms/android/library/src/main/java/io/element/android/wysiwyg/utils/HtmlToSpansParser.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ import io.element.android.wysiwyg.view.spans.PlainAtRoomMentionDisplaySpan
import io.element.android.wysiwyg.view.spans.QuoteSpan
import io.element.android.wysiwyg.view.spans.UnorderedListSpan
import org.jsoup.Jsoup
import org.jsoup.internal.StringUtil
import org.jsoup.nodes.Document.OutputSettings
import org.jsoup.nodes.Element
import org.jsoup.nodes.Node
import org.jsoup.nodes.TextNode
import org.jsoup.safety.Safelist
import timber.log.Timber
import kotlin.math.roundToInt

Expand All @@ -44,11 +48,20 @@ internal class HtmlToSpansParser(
private val mentionDisplayHandler: MentionDisplayHandler?,
private val isMention: ((text: String, url: String) -> Boolean)? = null,
) {
private val safeList = Safelist()
.addTags(
"a", "b", "strong", "i", "em", "u", "del", "code", "ul", "ol", "li", "pre",
"blockquote", "p", "br"
)
.addAttributes("a", "href", "data-mention-type", "contenteditable")

/**
* Convert the HTML string into a [Spanned] text.
*/
fun convert(): Spanned {
val dom = Jsoup.parse(html)
val outputSettings = OutputSettings().prettyPrint(false).indentAmount(0)
val cleanHtml = Jsoup.clean(html, "", safeList, outputSettings)
val dom = Jsoup.parse(cleanHtml)
val text = buildSpannedString {
val body = dom.body()
parseChildren(body)
Expand All @@ -58,11 +71,11 @@ internal class HtmlToSpansParser(
return text
}

private fun SpannableStringBuilder.parseChildren(element: Element) {
private fun SpannableStringBuilder.parseChildren(element: Element, parseTextNodes: Boolean = true) {
for (child in element.childNodes()) {
when (child) {
is Element -> parseElement(child)
is TextNode -> parseTextNode(child)
is TextNode -> if (parseTextNodes) parseTextNode(child)
}
}
}
Expand All @@ -74,8 +87,9 @@ internal class HtmlToSpansParser(
"i", "em" -> parseInlineFormatting(element, InlineFormat.Italic)
"u" -> parseInlineFormatting(element, InlineFormat.Underline)
"del" -> parseInlineFormatting(element, InlineFormat.StrikeThrough)
// Note we're using a different method for inline code
"code" -> parseInlineCode(element)
"ul", "ol" -> parseChildren(element)
"ul", "ol" -> parseList(element)
"li" -> parseListItem(element)
"pre" -> parseCodeBlock(element)
"blockquote" -> parseQuote(element)
Expand All @@ -89,8 +103,22 @@ internal class HtmlToSpansParser(

// region: Handle parsing of tags into spans

private fun SpannableStringBuilder.parseList(element: Element) {
addLeadingLineBreakForBlockNode(element)
parseChildren(element, parseTextNodes = false)
}

private fun SpannableStringBuilder.parseTextNode(child: TextNode) {
val text = child.wholeText
val isPreformattedText = child.anyAncestor { it.nameIs("pre") }
val text = if (isPreformattedText) {
child.wholeText
} else {
if (child.isBlank) {
child.normalisedWhitespace(stripLeading = true)
} else {
child.normalisedWhitespace(stripLeading = false)
}
}
if (text.isEmpty()) return

val previousSibling = child.previousSibling() as? Element
Expand All @@ -106,6 +134,7 @@ internal class HtmlToSpansParser(
InlineFormat.Italic -> StyleSpan(Typeface.ITALIC)
InlineFormat.Underline -> UnderlineSpan()
InlineFormat.StrikeThrough -> StrikethroughSpan()
// This is handled in parseInlineCode instead
InlineFormat.InlineCode -> return
}
inSpans(span) {
Expand All @@ -130,19 +159,20 @@ internal class HtmlToSpansParser(
private fun SpannableStringBuilder.parseQuote(element: Element) {
addLeadingLineBreakForBlockNode(element)
val start = this.length
inSpans(
inSpansWithFlags(
QuoteSpan(
// TODO provide these values from the style config
indicatorColor = 0xC0A0A0A0.toInt(),
indicatorWidth = 4.dpToPx().toInt(),
indicatorPadding = 6.dpToPx().toInt(),
margin = 10.dpToPx().toInt(),
)
),
// Used to blockquote always wraps any internal block element (list, code block, etc.)
flags = Spanned.SPAN_INCLUSIVE_EXCLUSIVE or (1 shl Spanned.SPAN_PRIORITY_SHIFT)
) {
parseChildren(element)
handleNbspInBlock(element, start, length)
}

handleNbspInBlock(element, start, length)
}

private fun SpannableStringBuilder.parseCodeBlock(element: Element) {
Expand All @@ -156,9 +186,9 @@ internal class HtmlToSpansParser(
)
) {
append(element.wholeText())
handleNbspInBlock(element, start, length)
}

handleNbspInBlock(element, start, length)
// Handle NBSPs for new lines inside the preformatted text
for (i in start + 1 until length) {
if (this[i] == NBSP) {
Expand All @@ -180,12 +210,13 @@ internal class HtmlToSpansParser(
val order = (element.parent()?.select("li")?.indexOf(element) ?: 0) + 1
OrderedListSpan(typeface, textSize, order, gapWidth)
}

else -> return
}
addLeadingLineBreakForBlockNode(element)
val start = this.length
inSpans(span) {
parseChildren(element)
handleNbspInBlock(element, start, length)
}
}

Expand Down Expand Up @@ -251,7 +282,7 @@ internal class HtmlToSpansParser(
private fun SpannableStringBuilder.handleNbspInBlock(element: Element, start: Int, end: Int) {
if (!element.isBlock) return

if (element.childNodes().isEmpty()) {
if (element.childNodes().isEmpty() && this.isNotEmpty()) {
this.append(NBSP)
setSpan(ExtraCharacterSpan(), end - 1, end, Spanned.SPAN_INCLUSIVE_EXCLUSIVE)
} else if (end - start == 1 && this.getOrNull(start) in listOf(' ', NBSP)) {
Expand All @@ -260,7 +291,7 @@ internal class HtmlToSpansParser(
}

private fun SpannableStringBuilder.addLeadingLineBreakForBlockNode(element: Element) {
if (element.isBlock && element.previousElementSibling() != null) {
if (element.isBlock && element.previousElementSibling()?.takeIf { it.tagName() != "br" } != null) {
append('\n')
}
}
Expand Down Expand Up @@ -291,6 +322,57 @@ internal class HtmlToSpansParser(
}
}

private fun Node.anyAncestor(block: (Node) -> Boolean): Boolean {
var parent = parent()
while (parent != null) {
if (block(parent)) return true
parent = parent.parent()
}
return false
}

private fun TextNode.normalisedWhitespace(stripLeading: Boolean): String {
var lastWasWhite = false
var reachedNonWhite = false
val text = wholeText
// Special case for when there's a single space
if (stripLeading && wholeText == " ") return wholeText
val result = StringUtil.borrowBuilder()
var i = 0
while (i < wholeText.length) {
val c = text.codePointAt(i)
if (StringUtil.isActuallyWhitespace(c)) {
if (c == NBSP.code) {
result.appendCodePoint(c)
} else if ((stripLeading && !reachedNonWhite) || lastWasWhite) {
i += Character.charCount(c)
continue
} else {
result.append(' ')
}
lastWasWhite = true
} else {
result.appendCodePoint(c)
reachedNonWhite = true
lastWasWhite = false
}
i += Character.charCount(c)
}
return StringUtil.releaseBuilder(result)
}

private inline fun SpannableStringBuilder.inSpansWithFlags(
vararg spans: Any,
flags: Int,
block: SpannableStringBuilder.() -> Unit
) {
val from = length
block()
val to = length
for (span in spans) {
setSpan(span, from, to, flags)
}
}

companion object FormattingSpans {
/**
Expand Down
43 changes: 19 additions & 24 deletions ...android/library/src/test/kotlin/io/element/android/wysiwyg/utils/HtmlToSpansParserTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ class HtmlToSpansParserTest {
assertThat(
spanned.dumpSpans(), equalTo(
listOf(
"bold: android.text.style.StyleSpan (0-4) fl=#33",
"italic: android.text.style.StyleSpan (4-10) fl=#33",
"underline: android.text.style.UnderlineSpan (10-19) fl=#33",
"strong: android.text.style.StyleSpan (19-25) fl=#33",
"emphasis: android.text.style.StyleSpan (25-33) fl=#33",
"strikethrough: android.text.style.StrikethroughSpan (33-46) fl=#33",
"code: io.element.android.wysiwyg.view.spans.InlineCodeSpan (46-50) fl=#33",
"bold: android.text.style.StyleSpan (0-4) fl=#17",
"italic: android.text.style.StyleSpan (4-10) fl=#17",
"underline: android.text.style.UnderlineSpan (10-19) fl=#17",
"strong: android.text.style.StyleSpan (19-25) fl=#17",
"emphasis: android.text.style.StyleSpan (25-33) fl=#17",
"strikethrough: android.text.style.StrikethroughSpan (33-46) fl=#17",
"code: io.element.android.wysiwyg.view.spans.InlineCodeSpan (46-50) fl=#17",
)
)
)
Expand All @@ -56,14 +56,13 @@ class HtmlToSpansParserTest {
""".trimIndent()
val spanned = convertHtml(html)


assertThat(
spanned.dumpSpans().joinToString(",\n"), equalTo(
"""
ordered1: io.element.android.wysiwyg.view.spans.OrderedListSpan (0-8) fl=#34,
ordered2: io.element.android.wysiwyg.view.spans.OrderedListSpan (9-17) fl=#34,
bullet1: io.element.android.wysiwyg.view.spans.UnorderedListSpan (18-25) fl=#34,
bullet2: io.element.android.wysiwyg.view.spans.UnorderedListSpan (26-33) fl=#34
ordered1: io.element.android.wysiwyg.view.spans.OrderedListSpan (0-8) fl=#17,
ordered2: io.element.android.wysiwyg.view.spans.OrderedListSpan (9-17) fl=#17,
bullet1: io.element.android.wysiwyg.view.spans.UnorderedListSpan (18-25) fl=#17,
bullet2: io.element.android.wysiwyg.view.spans.UnorderedListSpan (26-33) fl=#17
""".trimIndent()
)
)
Expand Down Expand Up @@ -104,13 +103,12 @@ class HtmlToSpansParserTest {
assertThat(
spanned.dumpSpans(), equalTo(
listOf(
"$NBSP: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (0-1) fl=#17",
"$NBSP: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (2-3) fl=#17"
"\n: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (0-1) fl=#17",
)
)
)
assertThat(
spanned.toString(), equalTo("$NBSP\n$NBSP")
spanned.toString(), equalTo("\n$NBSP")
)
}

Expand All @@ -119,9 +117,7 @@ class HtmlToSpansParserTest {
val html = "<p>Hello</p><br /><p>world</p>"
val spanned = convertHtml(html)
assertThat(
spanned.dumpSpans(), equalTo(
emptyList()
)
spanned.dumpSpans(), equalTo(emptyList())
)
assertThat(
spanned.toString(), equalTo("Hello\n\nworld")
Expand All @@ -131,9 +127,8 @@ class HtmlToSpansParserTest {
@Test
fun testMentionDisplayWithCustomMentionDisplayHandler() {
val html = """
<a href="https://element.io">link</a>
<a href="https://matrix.to/#/@test:example.org" contenteditable="false">jonny</a>
@room
<a href="https://element.io">link</a>$NBSP
<a href="https://matrix.to/#/@test:example.org" contenteditable="false">jonny</a>$NBSP@room
""".trimIndent()
val spanned = convertHtml(html, mentionDisplayHandler = object : MentionDisplayHandler {
override fun resolveAtRoomMentionDisplay(): TextDisplay =
Expand All @@ -145,15 +140,15 @@ class HtmlToSpansParserTest {
assertThat(
spanned.dumpSpans(), equalTo(
listOf(
"link: io.element.android.wysiwyg.view.spans.LinkSpan (0-4) fl=#33",
"jonny: io.element.android.wysiwyg.view.spans.PillSpan (5-10) fl=#33",
"link: io.element.android.wysiwyg.view.spans.LinkSpan (0-4) fl=#17",
"onny: io.element.android.wysiwyg.view.spans.ExtraCharacterSpan (6-10) fl=#33",
"jonny: io.element.android.wysiwyg.view.spans.PillSpan (5-10) fl=#17",
"@room: io.element.android.wysiwyg.view.spans.PillSpan (11-16) fl=#33",
)
)
)
assertThat(
spanned.toString(), equalTo("link\njonny\n@room")
spanned.toString().replace(NBSP, ' '), equalTo("link jonny @room")
)
}

Expand Down

0 comments on commit 5cfb891

Please sign in to comment.