diff --git a/CHANGELOG.md b/CHANGELOG.md index c90cc126cc70..75be9c9fbb0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ version code per locale under `fastlane/metadata/android//changelogs/`, written by `scripts/store-copy.py` before the release and uploaded by it. Play takes 500 characters, so not everything here reaches the store. +## Unreleased + +- A spreadsheet too big to show in full says so, and names how many of its rows + and columns are on screen. It used to stop without a word. +- How much of a sheet is shown follows the device's memory now, rather than one + number for every phone. A big sheet used to take the app past what the phone + could hold and fail to open at all. + ## 4.19.1 - The app no longer closes when the screen is rotated while an advertisement is diff --git a/app/src/androidTest/java/app/opendocument/droid/background/DocumentParcelTest.kt b/app/src/androidTest/java/app/opendocument/droid/background/DocumentParcelTest.kt index 1bc2662f7707..0e996f45f478 100644 --- a/app/src/androidTest/java/app/opendocument/droid/background/DocumentParcelTest.kt +++ b/app/src/androidTest/java/app/opendocument/droid/background/DocumentParcelTest.kt @@ -104,6 +104,8 @@ class DocumentParcelTest { Uri.parse("http://localhost:29665/file/odr/1.html"), Uri.parse("http://localhost:29665/file/odr/2.html"), ), + // the middle sheet is the only one the budget cut + listOf(null, SheetCut(80000, 12, 8333, 12), null), isEditable = true, readsAsDocument = true, ) @@ -116,6 +118,17 @@ class DocumentParcelTest { assertEquals(document.partUris, restored.partUris) assertTrue(restored.isEditable) assertTrue(restored.readsAsDocument) + + assertNull(restored.partCuts[0]) + assertNull(restored.partCuts[2]) + + val cut = checkNotNull(restored.partCuts[1]) + assertEquals(80000, cut.contentRows) + assertEquals(12, cut.contentColumns) + assertEquals(8333, cut.renderedRows) + assertEquals(12, cut.renderedColumns) + assertTrue(cut.rowsWereCut) + assertEquals(false, cut.columnsWereCut) } /** Everything but a spreadsheet: one part, and the core does not name it. */ @@ -133,6 +146,7 @@ class DocumentParcelTest { ), listOf(null), listOf(Uri.parse("http://localhost:29665/file/odr/document.html")), + listOf(null), isEditable = false, readsAsDocument = true, ), @@ -141,6 +155,7 @@ class DocumentParcelTest { assertEquals(1, restored.partTitles.size) assertNull(restored.partTitles[0]) + assertNull(restored.partCuts[0]) assertEquals(false, restored.isEditable) assertTrue(restored.readsAsDocument) } diff --git a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt index 7c5739120214..ee7e1b2a6ae4 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt @@ -6,6 +6,7 @@ import androidx.test.platform.app.InstrumentationRegistry import app.opendocument.core.FileType import app.opendocument.core.OdrException import app.opendocument.droid.background.CoreLoader +import app.opendocument.droid.background.SpreadsheetBudget import app.opendocument.droid.nonfree.CrashManager import java.io.File import java.io.FileOutputStream @@ -286,6 +287,48 @@ class CoreTest { return URL(views.first().url).readText() } + /** + * A cut sheet says how much of it was written. Generated rather than shipped, since + * `SpreadsheetBudget` answers per device. + */ + @Test + fun aSheetPastTheBudgetSaysWhatItLeftOut() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val budget = SpreadsheetBudget.cells(context) + + val columns = 10 + val expectedRows = (budget / columns).toInt() + val rows = expectedRows + 500 + + val views = + coreLoader.host( + prefix = "big-sheet", + inputPath = generateCsv(rows, columns).absolutePath, + cachePath = File(cacheDir(), "big_sheet_cache").path, + ) + + val cut = + checkNotNull(views.first().sheetCut) { "a sheet past the budget should report a cut" } + + Assert.assertEquals("every row written should be counted", rows, cut.contentRows) + Assert.assertEquals(columns, cut.contentColumns) + Assert.assertEquals("the budget decides the rows", expectedRows, cut.renderedRows) + Assert.assertEquals("a narrow sheet loses no columns", columns, cut.renderedColumns) + } + + /** The other side of it: a sheet written whole reports nothing to say. */ + @Test + fun aSheetInsideTheBudgetReportsNoCut() { + val views = + coreLoader.host( + prefix = "whole-sheet", + inputPath = spreadsheetTestFile.absolutePath, + cachePath = File(cacheDir(), "whole_sheet_cache").path, + ) + + views.forEach { Assert.assertNull("nothing was cut from " + it.name, it.sheetCut) } + } + @Test fun testSpreadsheetSheetNames() { val views = @@ -372,6 +415,31 @@ class CoreTest { private fun cacheDir(): File = InstrumentationRegistry.getInstrumentation().targetContext.cacheDir + /** A csv of [rows] x [columns] cells, each one a short string. */ + private fun generateCsv(rows: Int, columns: Int): File { + val target = File(cacheDir(), "generated-sheet.csv") + + target.bufferedWriter().use { writer -> + for (row in 1..rows) { + for (column in 1..columns) { + if (column > 1) { + writer.write(",") + } + + writer.write("r") + writer.write(row.toString()) + writer.write("c") + writer.write(column.toString()) + } + + writer.write("\n") + } + } + extracted += target + + return target + } + private fun extract(name: String): File { val instrumentation = InstrumentationRegistry.getInstrumentation() val target = File(instrumentation.targetContext.cacheDir, name) diff --git a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt index f025d4734c9b..ca2d74ae83f5 100644 --- a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt +++ b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt @@ -99,6 +99,7 @@ class CoreLoader(private val context: Context) { file, views.map { it.name }, views.map { Uri.parse(it.url) }, + views.map { it.sheetCut }, isDocumentEditable, readsAsDocument, ) @@ -186,10 +187,10 @@ class CoreLoader(private val context: Context) { // document. PageView.setDarkeningAllowed picks between them htmlConfig.colorScheme = HtmlColorScheme.SYSTEM - // stated rather than inherited: a sheet past it is cut off silently + // stated rather than inherited, and this device's answer - see SpreadsheetBudget htmlConfig.spreadsheetLimit = - TableDimensions(SPREADSHEET_LIMIT_ROWS, SPREADSHEET_LIMIT_COLUMNS) - htmlConfig.spreadsheetCellLimit = SPREADSHEET_LIMIT_CELLS + TableDimensions(SpreadsheetBudget.ROWS, SpreadsheetBudget.COLUMNS) + htmlConfig.spreadsheetCellLimit = SpreadsheetBudget.cells(context) htmlConfig.spreadsheetLimitByContent = true val cacheDirectory = File(cachePath) @@ -204,6 +205,14 @@ class CoreLoader(private val context: Context) { HostedView( view.name(), "http://$SERVER_URL_HOST:$sharedServerPort/file/$prefix/" + view.path(), + view.sheetCut()?.let { + SheetCut( + it.content.rows, + it.content.columns, + it.rendered.rows, + it.rendered.columns, + ) + }, ) } } @@ -330,8 +339,11 @@ class CoreLoader(private val context: Context) { document = null } - /** A translated view of a document, ready to be opened in the WebView. */ - data class HostedView(val name: String, val url: String) + /** + * A translated view of a document, ready to be opened in the WebView. [sheetCut] is set only + * where the budget cut this view's sheet. + */ + data class HostedView(val name: String, val url: String, val sheetCut: SheetCut?) /** An encrypted file whose format odrcore cannot decrypt, whatever the password. */ class UndecryptableFile(path: String) : IOException("cannot be decrypted: $path") @@ -339,13 +351,6 @@ class CoreLoader(private val context: Context) { companion object { private const val TAG = "CoreLoader" - /** The largest sheet region translated - every cell in it becomes a ``. */ - private const val SPREADSHEET_LIMIT_ROWS = 100000 - private const val SPREADSHEET_LIMIT_COLUMNS = 500 - - /** Bounds the rows by the sheet's width. */ - private const val SPREADSHEET_LIMIT_CELLS = 500000L - /** * The one http server of the process, started on the first [initialize] and never stopped. * diff --git a/app/src/main/java/app/opendocument/droid/background/LoadedDocument.kt b/app/src/main/java/app/opendocument/droid/background/LoadedDocument.kt index 283a3fdc9d97..aae89ca133d6 100644 --- a/app/src/main/java/app/opendocument/droid/background/LoadedDocument.kt +++ b/app/src/main/java/app/opendocument/droid/background/LoadedDocument.kt @@ -9,6 +9,8 @@ import android.os.Parcelable * be, and one uri per part (spreadsheets have one per sheet, everything else a single one with a * null title). * + * [partCuts] runs alongside them, null for every part but a sheet that was cut. + * * [isEditable] and [readsAsDocument] are the core's own answers about this document, never a guess * from its mime type - see `CoreLoader.isDocumentEditable` and `CoreLoader.readsAsDocument`. */ @@ -17,6 +19,7 @@ class LoadedDocument( val file: IdentifiedFile, val partTitles: List, val partUris: List, + val partCuts: List, val isEditable: Boolean, val readsAsDocument: Boolean, ) : Parcelable { @@ -28,6 +31,7 @@ class LoadedDocument( parcel.writeParcelable(file, 0) parcel.writeList(partTitles) parcel.writeList(partUris) + parcel.writeList(partCuts) ParcelUtil.writeBoolean(parcel, isEditable) ParcelUtil.writeBoolean(parcel, readsAsDocument) } @@ -51,11 +55,15 @@ class LoadedDocument( val partUris = ArrayList() parcel.readList(partUris, classLoader) + val partCuts = ArrayList() + parcel.readList(partCuts, classLoader) + return LoadedDocument( request, file, partTitles, partUris, + partCuts, ParcelUtil.readBoolean(parcel), ParcelUtil.readBoolean(parcel), ) diff --git a/app/src/main/java/app/opendocument/droid/background/SheetCut.kt b/app/src/main/java/app/opendocument/droid/background/SheetCut.kt new file mode 100644 index 000000000000..6f837538934e --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/SheetCut.kt @@ -0,0 +1,48 @@ +package app.opendocument.droid.background + +import android.os.Parcel +import android.os.Parcelable + +/** + * How much of a sheet the markup carries against how much was written - odrcore's own + * `HtmlView.sheetCut()`, and only present where [SpreadsheetBudget] cut the sheet. + */ +class SheetCut( + val contentRows: Int, + val contentColumns: Int, + val renderedRows: Int, + val renderedColumns: Int, +) : Parcelable { + + val rowsWereCut: Boolean + get() = renderedRows < contentRows + + val columnsWereCut: Boolean + get() = renderedColumns < contentColumns + + override fun describeContents(): Int = 0 + + override fun writeToParcel(parcel: Parcel, flags: Int) { + parcel.writeInt(contentRows) + parcel.writeInt(contentColumns) + parcel.writeInt(renderedRows) + parcel.writeInt(renderedColumns) + } + + companion object { + // @JvmField because the framework looks CREATOR up as a static field + @JvmField + val CREATOR: Parcelable.Creator = + object : Parcelable.Creator { + override fun createFromParcel(parcel: Parcel): SheetCut = + SheetCut( + parcel.readInt(), + parcel.readInt(), + parcel.readInt(), + parcel.readInt(), + ) + + override fun newArray(size: Int): Array = arrayOfNulls(size) + } + } +} diff --git a/app/src/main/java/app/opendocument/droid/background/SpreadsheetBudget.kt b/app/src/main/java/app/opendocument/droid/background/SpreadsheetBudget.kt new file mode 100644 index 000000000000..03856d8c4ecc --- /dev/null +++ b/app/src/main/java/app/opendocument/droid/background/SpreadsheetBudget.kt @@ -0,0 +1,46 @@ +package app.opendocument.droid.background + +import android.app.ActivityManager +import android.content.Context +import androidx.core.content.getSystemService + +/** + * How much of a sheet is translated to html. + * + * The budget is the *WebView's*, not the core's: a rendered cell costs 10-20 KB in the renderer + * process against some 226 bytes of html, so a budget too high shows none of the document rather + * than more of it - the page fails to load and the file is reported as one that cannot be opened. + */ +object SpreadsheetBudget { + + /** Each direction on its own, before [cells] narrows the two together. */ + const val ROWS = 100000 + const val COLUMNS = 500 + + /** What the device this is running on can afford to show. */ + fun cells(context: Context): Long { + val activityManager = context.getSystemService() + val memoryInfo = ActivityManager.MemoryInfo() + activityManager?.getMemoryInfo(memoryInfo) + + return cellsFor(memoryInfo.totalMem, activityManager?.isLowRamDevice == true) + } + + /** + * Memory decides, since what is budgeted is the renderer process. [totalMemoryBytes] of zero is + * a device that would not answer, and takes the smallest step. + */ + fun cellsFor(totalMemoryBytes: Long, isLowRamDevice: Boolean): Long { + if (isLowRamDevice || totalMemoryBytes < 3L * GIGABYTE) { + return 50000 + } + + if (totalMemoryBytes < 6L * GIGABYTE) { + return 100000 + } + + return 150000 + } + + private const val GIGABYTE = 1024L * 1024L * 1024L +} diff --git a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt b/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt index 4fca9f7ff608..d45bda58cbcf 100644 --- a/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt +++ b/app/src/main/java/app/opendocument/droid/ui/SnackbarHelper.kt @@ -65,6 +65,24 @@ object SnackbarHelper { ) } + /** Same, where the message carries numbers and is built rather than looked up. */ + fun show( + activity: Activity, + message: String, + callback: Runnable?, + isIndefinite: Boolean, + isError: Boolean, + ) { + show( + activity, + activity.getString(android.R.string.ok), + message, + callback, + isIndefinite, + isError, + ) + } + private fun show( activity: Activity, buttonText: String, diff --git a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt index 9d1d8e3354fa..78a118486489 100644 --- a/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt +++ b/app/src/main/java/app/opendocument/droid/ui/activity/DocumentFragment.kt @@ -34,6 +34,7 @@ import app.opendocument.droid.background.LoadedDocument import app.opendocument.droid.background.NightModeSetting import app.opendocument.droid.background.PaginationSetting import app.opendocument.droid.background.ReviewInvitation +import app.opendocument.droid.background.SheetCut import app.opendocument.droid.nonfree.AnalyticsConstants import app.opendocument.droid.nonfree.AnalyticsManager import app.opendocument.droid.nonfree.CrashManager @@ -44,6 +45,7 @@ import app.opendocument.droid.ui.widget.PageView import app.opendocument.droid.ui.widget.ProgressDialogFragment import com.google.android.material.tabs.TabLayout import java.io.FileNotFoundException +import java.text.NumberFormat class DocumentFragment : Fragment(), DocumentLoader.Listener { @@ -83,6 +85,12 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener { */ private var positionToRestore: ReadingPosition? = null + /** + * Whether the bar on show is [reportSheetCut]'s, so moving to a whole sheet takes that one down + * and leaves any other alone. + */ + private var sheetCutReported = false + /** A tab and how far down it, which survives the document being translated again. */ private data class ReadingPosition(val tab: Int, val scrollFraction: Float) @@ -705,6 +713,9 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener { tabLayout.getTabAt(restored?.tab?.coerceAtMost(pages - 1) ?: 0)?.select() } else if (pages == 1) { loadData(document.partUris[0].toString()) + + // with tabs it is the tab listener that says this + reportSheetCut(document, 0) } prepareActions(document) @@ -982,6 +993,61 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener { ) } + /** + * Names what the sheet on screen leaves out. Nothing is offered beyond the numbers: the budget + * is what the WebView can hold, not a preference. + */ + private fun reportSheetCut(document: LoadedDocument, part: Int) { + val cut = document.partCuts.getOrNull(part) + + if (cut == null) { + if (sheetCutReported) { + sheetCutReported = false + + SnackbarHelper.dismiss(requireActivity()) + } + + return + } + + SnackbarHelper.show( + requireActivity(), + describeSheetCut(cut), + null, + isIndefinite = false, + isError = false, + ) + + sheetCutReported = true + } + + private fun describeSheetCut(cut: SheetCut): String { + val numbers = NumberFormat.getIntegerInstance() + + return when { + cut.rowsWereCut && cut.columnsWereCut -> + getString( + R.string.toast_hint_sheet_cut_rows_and_columns, + numbers.format(cut.renderedRows), + numbers.format(cut.contentRows), + numbers.format(cut.renderedColumns), + numbers.format(cut.contentColumns), + ) + cut.columnsWereCut -> + getString( + R.string.toast_hint_sheet_cut_columns, + numbers.format(cut.renderedColumns), + numbers.format(cut.contentColumns), + ) + else -> + getString( + R.string.toast_hint_sheet_cut_rows, + numbers.format(cut.renderedRows), + numbers.format(cut.contentRows), + ) + } + } + fun openWith(activity: Activity) { doReopen(activity, requireLastRequest(), state.lastFile, share = false) } @@ -1140,6 +1206,8 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener { state.lastSelectedTab = tab.position loadData(lastDocument.partUris[tab.position].toString()) + + reportSheetCut(lastDocument, tab.position) } override fun onTabUnselected(tab: TabLayout.Tab) {} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index bc1c7ffd8740..13c3b0850c12 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -8,6 +8,10 @@ The file may be damaged, or something went wrong on our side. If you think the file is fine, tell us at support@opendocument.app Couldn\'t find file. Maybe it doesn\'t exist any longer? Unsupported file format. Try opening it in another app. + + This sheet is too big to show in full. Showing the first %1$s of %2$s rows. + This sheet is too big to show in full. Showing the first %1$s of %2$s columns. + This sheet is too big to show in full. Showing the first %1$s of %2$s rows and %3$s of %4$s columns. Not happy with how the file is displayed? Open it in another app instead. No place to save the file to chosen. File could not be saved. Please contact support@opendocument.app diff --git a/app/src/test/java/app/opendocument/droid/background/SpreadsheetBudgetTest.kt b/app/src/test/java/app/opendocument/droid/background/SpreadsheetBudgetTest.kt new file mode 100644 index 000000000000..625789e8f95c --- /dev/null +++ b/app/src/test/java/app/opendocument/droid/background/SpreadsheetBudgetTest.kt @@ -0,0 +1,48 @@ +package app.opendocument.droid.background + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** [SpreadsheetBudget.cellsFor] takes the memory, so every step is reachable without the phones. */ +class SpreadsheetBudgetTest { + + @Test + fun aSmallDeviceGetsTheSmallestBudget() { + assertEquals(50000L, cellsFor(gigabytes = 2)) + assertEquals(50000L, cellsFor(gigabytes = 8, isLowRamDevice = true)) + } + + @Test + fun anOrdinaryPhoneGetsTheMiddleOne() { + assertEquals(100000L, cellsFor(gigabytes = 4)) + } + + @Test + fun aLargeDeviceGetsTheLargest() { + assertEquals(150000L, cellsFor(gigabytes = 8)) + assertEquals(150000L, cellsFor(gigabytes = 16)) + } + + /** A device that would not say how much memory it has is treated as one that has little. */ + @Test + fun anUnansweredDeviceGetsTheSmallestBudget() { + assertEquals(50000L, cellsFor(gigabytes = 0)) + } + + /** The budget only ever grows with the memory: a step in the wrong place would show less. */ + @Test + fun theLadderNeverFalls() { + var previous = 0L + + for (gigabytes in 0..24) { + val cells = cellsFor(gigabytes) + + assert(cells >= previous) { "$gigabytes GB gets $cells, less than the device below it" } + + previous = cells + } + } + + private fun cellsFor(gigabytes: Int, isLowRamDevice: Boolean = false): Long = + SpreadsheetBudget.cellsFor(gigabytes * 1024L * 1024L * 1024L, isLowRamDevice) +}