From eabf243fc18cb7c82dbbef190e937ad67a3cf2da Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Sun, 16 Jun 2024 12:50:51 +0200 Subject: [PATCH] Added support for RTL tables Fixes #306 --- .../feeder/model/FullTextParser.kt | 13 +++++- .../feeder/model/html/HtmlLinearizer.kt | 6 ++- .../feeder/model/html/LinearStuff.kt | 44 ++++++++++++++++--- .../ui/compose/html/LinearArticleContent.kt | 11 ++--- .../feeder/ui/compose/layouts/Table.kt | 1 + .../ui/compose/text/HtmlToComposable.kt | 14 ++++++ .../feeder/model/html/HtmlLinearizerTest.kt | 29 ++++++++++-- 7 files changed, 99 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/nononsenseapps/feeder/model/FullTextParser.kt b/app/src/main/java/com/nononsenseapps/feeder/model/FullTextParser.kt index 4b838cd3d..8b614dd36 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/model/FullTextParser.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/model/FullTextParser.kt @@ -168,7 +168,18 @@ class FullTextParser(override val di: DI) : DIAware { uri: String, html: String, ): String? { - return Readability4JExtended(uri, html).parse().contentWithUtf8Encoding + val article = Readability4JExtended(uri, html).parse() + + val dir = article.dir + + // Ensure dir is set on the outermost element + return article.contentWithUtf8Encoding?.let { fullHtml -> + if (dir?.isNotBlank() == true) { + fullHtml.replaceFirst(" { finalizeAndAddCurrentElement(blockStyle) + // This can also be auto, but for tables that's equivalent to LTR probably + val leftToRight = element.attrInHierarchy("dir") != "rtl" + val rowSequence = sequence { element.children() @@ -556,7 +560,7 @@ class HtmlLinearizer { ) } else { add( - LinearTable.build { + LinearTable.build(leftToRight = leftToRight) { rowSequence .forEach { row -> newRow() diff --git a/app/src/main/java/com/nononsenseapps/feeder/model/html/LinearStuff.kt b/app/src/main/java/com/nononsenseapps/feeder/model/html/LinearStuff.kt index 0771fe4e0..57792da21 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/model/html/LinearStuff.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/model/html/LinearStuff.kt @@ -100,12 +100,24 @@ data class LinearTable( rowCount: Int, colCount: Int, cells: List, + leftToRight: Boolean, ) : this( rowCount, colCount, ArrayMap().apply { cells.forEachIndexed { index, item -> - put(Coordinate(row = index / colCount, col = index % colCount), item) + put( + Coordinate( + row = index / colCount, + col = + if (leftToRight) { + index % colCount + } else { + colCount - 1 - index % colCount + }, + ), + item, + ) } }, ) @@ -117,7 +129,7 @@ data class LinearTable( return cells[Coordinate(row = row, col = col)] } - class Builder { + class Builder(val leftToRight: Boolean) { private val cells: ArrayMap = ArrayMap() private var rowCount: Int = 0 private var colCount: Int = 0 @@ -165,13 +177,35 @@ data class LinearTable( } fun build(): LinearTable { - return LinearTable(rowCount, colCount, cells) + return LinearTable( + rowCount = rowCount, + colCount = colCount, + cellsReal = + if (leftToRight) { + cells + } else { + ArrayMap().apply { + cells.forEach { (ltrCoord, item) -> + put( + Coordinate( + row = ltrCoord.row, + col = colCount - 1 - ltrCoord.col, + ), + item, + ) + } + } + }, + ) } } companion object { - fun build(block: Builder.() -> Unit): LinearTable { - return Builder().apply(block).build() + fun build( + leftToRight: Boolean, + block: Builder.() -> Unit, + ): LinearTable { + return Builder(leftToRight = leftToRight).apply(block).build() } } } diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/html/LinearArticleContent.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/html/LinearArticleContent.kt index e807b886a..c9a5c54df 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/html/LinearArticleContent.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/html/LinearArticleContent.kt @@ -1115,6 +1115,7 @@ private fun PreviewLinearTableContent() { LinearTable( rowCount = 2, colCount = 2, + leftToRight = false, cells = listOf( LinearTableCellItem( @@ -1178,6 +1179,7 @@ private fun PreviewNestedTableContent() { LinearTable( rowCount = 2, colCount = 2, + leftToRight = false, cells = listOf( LinearTableCellItem( @@ -1268,6 +1270,7 @@ private fun PreviewNestedTableContent() { LinearTable( rowCount = 2, colCount = 2, + leftToRight = false, cells = listOf( LinearTableCellItem( @@ -1410,14 +1413,6 @@ fun LinearTable.toTableData(): TableData { }, ) } - .sortedWith( - compareBy( - { it.colSpan }, - { it.rowSpan }, - { it.row }, - { it.column }, - ), - ) .toList(), ) } diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/layouts/Table.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/layouts/Table.kt index 8f1af7d21..02eb32417 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/layouts/Table.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/layouts/Table.kt @@ -242,6 +242,7 @@ data class TableData private constructor( if (a.row != b.row) { return@sortedWith a.row.compareTo(b.row) } + return@sortedWith a.column.compareTo(b.column) }, ) diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt index 58cd24948..7f01226e1 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/text/HtmlToComposable.kt @@ -14,6 +14,20 @@ import org.jsoup.nodes.Element import org.jsoup.nodes.TextNode import kotlin.math.roundToInt +fun Element.attrInHierarchy(attr: String): String { + var current: Element? = this + + while (current != null) { + val value = current.attr(attr) + if (value.isNotEmpty()) { + return value + } + current = current.parent() + } + + return "" +} + fun Element.ancestors(predicate: (Element) -> Boolean): Sequence { return ancestors().filter(predicate) } diff --git a/app/src/test/java/com/nononsenseapps/feeder/model/html/HtmlLinearizerTest.kt b/app/src/test/java/com/nononsenseapps/feeder/model/html/HtmlLinearizerTest.kt index 413b9e891..5ff348bfe 100644 --- a/app/src/test/java/com/nononsenseapps/feeder/model/html/HtmlLinearizerTest.kt +++ b/app/src/test/java/com/nononsenseapps/feeder/model/html/HtmlLinearizerTest.kt @@ -714,7 +714,7 @@ class HtmlLinearizerTest { assertEquals(1, result.size, "Expected one item: $result") assertEquals( - LinearTable.build { + LinearTable.build(leftToRight = true) { newRow() add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("1", LinearTextBlockStyle.TEXT)))) add(LinearTableCellItem(type = LinearTableCellItemType.DATA, colSpan = 1, rowSpan = 1, content = listOf(LinearText("2", LinearTextBlockStyle.TEXT)))) @@ -726,6 +726,27 @@ class HtmlLinearizerTest { ) } + @Test + fun `table block 2x2 rtl`() { + val html = "
12
34
" + val baseUrl = "https://example.com" + + val result = linearizer.linearize(html, baseUrl).elements + + assertEquals(1, result.size, "Expected one item: $result") + assertEquals( + LinearTable.build(leftToRight = true) { + newRow() + add(LinearTableCellItem(type = LinearTableCellItemType.DATA, colSpan = 1, rowSpan = 1, content = listOf(LinearText("2", LinearTextBlockStyle.TEXT)))) + add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("1", LinearTextBlockStyle.TEXT)))) + newRow() + add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("4", LinearTextBlockStyle.TEXT)))) + add(LinearTableCellItem(type = LinearTableCellItemType.DATA, colSpan = 1, rowSpan = 1, content = listOf(LinearText("3", LinearTextBlockStyle.TEXT)))) + }, + result[0], + ) + } + @Test fun `table with colspan, rowspan, and a double span`() { val html = @@ -755,7 +776,7 @@ class HtmlLinearizerTest { assertEquals(1, result.size, "Expected one item: $result") val table = result[0] as LinearTable assertEquals( - LinearTable.build { + LinearTable.build(leftToRight = true) { newRow() add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Name", LinearTextBlockStyle.TEXT)))) add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Age", LinearTextBlockStyle.TEXT)))) @@ -798,7 +819,7 @@ class HtmlLinearizerTest { assertEquals(1, result.size, "Expected one item: $result") val table = result[0] as LinearTable assertEquals( - LinearTable.build { + LinearTable.build(leftToRight = true) { newRow() add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Name", LinearTextBlockStyle.TEXT)))) add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Age", LinearTextBlockStyle.TEXT)))) @@ -892,7 +913,7 @@ class HtmlLinearizerTest { assertEquals(1, result.size, "Expected one item: $result") val expected = - LinearTable.build { + LinearTable.build(leftToRight = true) { newRow() add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Name", LinearTextBlockStyle.TEXT)))) add(LinearTableCellItem(type = LinearTableCellItemType.HEADER, colSpan = 1, rowSpan = 1, content = listOf(LinearText("Number", LinearTextBlockStyle.TEXT))))