diff options
Diffstat (limited to 'app/src')
8 files changed, 396 insertions, 149 deletions
diff --git a/app/src/main/java/org/terst/feedreader/data/Settings.kt b/app/src/main/java/org/terst/feedreader/data/Settings.kt index ebeba6c..18c5f9d 100644 --- a/app/src/main/java/org/terst/feedreader/data/Settings.kt +++ b/app/src/main/java/org/terst/feedreader/data/Settings.kt @@ -39,12 +39,30 @@ private const val READER_FONT_SIZE_MIN_PX = 14 private const val READER_FONT_SIZE_MAX_PX = 28 val READER_FONT_SIZE_RANGE_PX = READER_FONT_SIZE_MIN_PX..READER_FONT_SIZE_MAX_PX +/** WebView's textZoom baseline: 100% is "render the CSS font sizes exactly as written." */ +const val UNZOOMED_TEXT_ZOOM_PERCENT = 100 + // Pure so the pinch-to-zoom math is testable without a WebView/gesture detector in the loop -- // ReaderWebView.kt calls this once per gesture (at pinch-end, with the cumulative scale factor // accumulated across the whole gesture), not per touch-move frame. fun pinchZoomFontSizePx(basePx: Int, scaleFactor: Float): Int = (basePx * scaleFactor).roundToInt().coerceIn(READER_FONT_SIZE_RANGE_PX.first, READER_FONT_SIZE_RANGE_PX.last) +/** + * The WebView textZoom percentage that makes text laid out at [renderedPx] read as [targetPx]. + * + * This is what makes a pinch reflow live instead of scaling the whole page and snapping back: + * WebView's textZoom re-lays-out the text (and only the text -- images and padding stay put) + * the moment it changes, with no reload and no JavaScript. + * + * [renderedPx] is the size the currently-loaded HTML was actually written at, which isn't always + * the saved font size -- a second pinch can start before the reload from the first one has + * landed. Measuring against the saved size in that window would shrink the text back to the + * old layout for a frame before growing again. + */ +fun readerTextZoomPercent(renderedPx: Int, targetPx: Int): Int = + (targetPx * UNZOOMED_TEXT_ZOOM_PERCENT / renderedPx.coerceAtLeast(1).toFloat()).roundToInt() + /** Stash connection settings, configured on the Settings screen. Both blank until set. */ class Settings(private val context: Context) { val stashBaseUrl: Flow<String> = diff --git a/app/src/main/java/org/terst/feedreader/ui/ImageZoom.kt b/app/src/main/java/org/terst/feedreader/ui/ImageZoom.kt index 262af87..bf03382 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ImageZoom.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ImageZoom.kt @@ -29,19 +29,6 @@ import androidx.compose.ui.window.DialogProperties import coil.compose.AsyncImage import kotlin.math.min -// Article images live inside the reader's WebView, which runs with JavaScript off (see -// ReaderWebView) -- so there's no in-page script that could notice a tap on an <img> and -// report it out. And WebView's own built-in zoom (setSupportZoom/builtInZoomControls) isn't -// an option either: pinching in the reader is already taken, deliberately, by the -// reader-font-size gesture, and page zoom would scale the whole article's layout rather than -// giving an image the full screen. -// -// So tap detection is done the one way that works with JS off: every image the reader renders -// is wrapped in a link to a scheme nothing can actually navigate to, which WebView then hands -// to shouldOverrideUrlLoading -- the same interception point that already routes ordinary -// links and YouTube thumbnails. The callback opens ImageZoomDialog instead of following it. -private const val IMAGE_ZOOM_SCHEME = "feedreader-image:" - /** Fitted (1x) up to this; past ~6x an article image is nothing but its own pixels. */ const val IMAGE_ZOOM_MAX_SCALE = 6f @@ -51,94 +38,6 @@ private const val IMAGE_ZOOM_DOUBLE_TAP_SCALE = 3f // instead. Far enough that an ordinary sloppy tap can't trigger it by accident. private val SWIPE_DISMISS_DISTANCE = 120.dp -// Matches an opening <a>, a closing </a>, or an <img> -- in document order, so a single pass -// can tell which images are already inside a link. `<a\b` deliberately doesn't match <abbr>, -// <article> etc., since \b requires a non-word character right after the "a". -private val anchorOrImageTagRegex = Regex("""<a\b[^>]*>|</a\s*>|<img\b[^>]*>""", RegexOption.IGNORE_CASE) - -private val imgSrcRegex = Regex("""\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)')""", RegexOption.IGNORE_CASE) - -private val imageFileExtensions = listOf(".jpg", ".jpeg", ".png", ".gif", ".webp", ".avif", ".bmp", ".heic", ".heif") - -/** - * Wraps every article image that isn't already inside a link in a zoom link, so tapping it - * reaches [zoomableImageTarget] via the reader WebView's shouldOverrideUrlLoading. - * - * Images that are already inside a link are left exactly as they were -- their link is doing - * something already (ReaderExtractor's synthesized YouTube thumbnail is precisely this shape, - * and wrapping it would swallow the tap that plays the video). Those are still zoomable when - * the link points at an image file, which [zoomableImageTarget] handles on the other side. - */ -fun linkifyImagesForZoom(html: String): String = buildString { - var copiedUpTo = 0 - var anchorDepth = 0 - for (tag in anchorOrImageTagRegex.findAll(html)) { - val text = tag.value - when { - text.startsWith("</", ignoreCase = true) -> if (anchorDepth > 0) anchorDepth-- - text.startsWith("<a", ignoreCase = true) -> anchorDepth++ - anchorDepth == 0 -> { - val src = imgSrcRegex.find(text)?.let { it.groupValues[1].ifEmpty { it.groupValues[2] } } - val href = src?.let { zoomHrefForSrc(it) } - if (href != null) { - append(html, copiedUpTo, tag.range.first) - append("<a href=\"").append(href).append("\">").append(text).append("</a>") - copiedUpTo = tag.range.last + 1 - } - } - } - } - append(html, copiedUpTo, html.length) -} - -private fun zoomHrefForSrc(src: String): String? { - val trimmed = src.trim() - val absolute = when { - // Protocol-relative (//host/path) is common in feed markup and resolves fine in the - // page itself, but our scheme-prefixed href has to carry a complete URL on its own. - trimmed.startsWith("//") -> "https:$trimmed" - trimmed.startsWith("http://", ignoreCase = true) || trimmed.startsWith("https://", ignoreCase = true) -> trimmed - // Anything else (site-relative, data:, javascript:) is skipped rather than guessed at: - // a relative src resolves against the WebView's base URL, which a custom-scheme href - // can't reproduce, so wrapping it would only build a dead link. - else -> null - } ?: return null - // The src came out of a quoted attribute and goes straight back into one, already escaped - // as the source wrote it. A raw quote or angle bracket could only arrive via a - // single-quoted attribute, and would break out of the href -- drop those instead. - return if (absolute.any { it == '"' || it == '<' || it == '>' }) null else IMAGE_ZOOM_SCHEME + absolute -} - -/** - * The image a tapped URL should open full-screen, or null if it isn't an image at all. - * - * Covers both halves of the problem: links [linkifyImagesForZoom] built, and links the article - * itself already had that point straight at an image file (WordPress's default "link to media - * file" wraps article images in exactly that, which used to send a tap off to LinkReaderScreen - * to try and run Readability over a JPEG). - */ -fun zoomableImageTarget(url: String): String? = when { - url.startsWith(IMAGE_ZOOM_SCHEME, ignoreCase = true) -> - url.substring(IMAGE_ZOOM_SCHEME.length).takeIf { it.isNotBlank() } - looksLikeImageUrl(url) -> url - else -> null -} - -// Extension-sniffing, not a content-type check -- there's no response to inspect at the point -// a link is tapped. SVG is deliberately not in the list: Coil's default decoders can't render -// one, so a viewer opened on it would just show an empty black screen. -private fun looksLikeImageUrl(url: String): Boolean { - val path = url.substringBefore('#').substringBefore('?').lowercase() - return imageFileExtensions.any { path.endsWith(it) } -} - -/** - * The URL to show the user for a link -- the zoom scheme is an internal detail, so a - * long-press on a zoom-wrapped image offers actions on the real image URL. - */ -fun withoutImageZoomScheme(url: String): String = - if (url.startsWith(IMAGE_ZOOM_SCHEME, ignoreCase = true)) url.substring(IMAGE_ZOOM_SCHEME.length) else url - /** Applies a pinch to the current zoom level, clamped to fitted-size..[IMAGE_ZOOM_MAX_SCALE]. */ fun imageZoomScale(currentScale: Float, gestureZoom: Float): Float = (currentScale * gestureZoom).coerceIn(1f, IMAGE_ZOOM_MAX_SCALE) @@ -179,6 +78,50 @@ fun fittedImageSize( return imageWidth * fit to imageHeight * fit } +/** How the viewer is currently showing the image: zoom level, plus where it's been dragged to. */ +data class ImageZoomState(val scale: Float = 1f, val offsetX: Float = 0f, val offsetY: Float = 0f) + +/** + * The viewer's state after one step of a transform gesture -- the whole pinch-and-pan + * calculation in one place, rather than spread across the gesture callback. + * + * It lives here, as a function of nothing but its arguments, because the callback can't safely + * close over anything computed during composition: a `pointerInput(Unit)` block is created once + * and keeps running with that first lambda, so a pan bound computed outside it stayed frozen at + * its first-composition value -- a zero-size viewport holding a not-yet-decoded image, whose + * only valid offset is 0. That's the whole reason a zoomed image wouldn't pan. Everything the + * math needs is a parameter, so the callback has to read it fresh each time. + */ +fun imageZoomTransform( + state: ImageZoomState, + viewportSize: Pair<Float, Float>, + intrinsicSize: Pair<Float, Float>, + pan: Pair<Float, Float>, + gestureZoom: Float +): ImageZoomState { + val scale = imageZoomScale(state.scale, gestureZoom) + val (fittedWidth, fittedHeight) = + fittedImageSize(viewportSize.first, viewportSize.second, intrinsicSize.first, intrinsicSize.second) + return ImageZoomState( + scale = scale, + offsetX = clampImagePanOffset(state.offsetX + pan.first, viewportSize.first, fittedWidth, scale), + offsetY = clampImagePanOffset(state.offsetY + pan.second, viewportSize.second, fittedHeight, scale) + ) +} + +/** Double-tap: toggle the zoom level, re-clamping the existing pan to whatever it becomes. */ +fun imageZoomDoubleTap( + state: ImageZoomState, + viewportSize: Pair<Float, Float>, + intrinsicSize: Pair<Float, Float> +): ImageZoomState = imageZoomTransform( + state = state, + viewportSize = viewportSize, + intrinsicSize = intrinsicSize, + pan = 0f to 0f, + gestureZoom = imageZoomDoubleTapScale(state.scale) / state.scale +) + /** * Full-screen viewer for a tapped article image: pinch to zoom, drag to pan while zoomed, * double-tap to toggle between fitted and zoomed, and dismiss with the system back gesture, @@ -194,16 +137,18 @@ fun ImageZoomDialog(imageUrl: String, onDismiss: () -> Unit) { // usePlatformDefaultWidth would cap this at the platform's narrow AlertDialog width -- // same reason YouTubePlayerDialog opts out. A photo viewer wants the whole screen. Dialog(onDismissRequest = onDismiss, properties = DialogProperties(usePlatformDefaultWidth = false)) { - var scale by remember { mutableStateOf(1f) } - var offsetX by remember { mutableStateOf(0f) } - var offsetY by remember { mutableStateOf(0f) } + var zoom by remember(imageUrl) { mutableStateOf(ImageZoomState()) } var viewport by remember { mutableStateOf(IntSize.Zero) } var dismissDragY by remember { mutableStateOf(0f) } // The image's own decoded dimensions, reported by Coil once it has one to report. // Panning is bounded by where the image actually is, not by the whole viewport -- // ContentScale.Fit letterboxes anything that isn't exactly the screen's aspect ratio. var intrinsic by remember(imageUrl) { mutableStateOf(0f to 0f) } - val fitted = fittedImageSize(viewport.width.toFloat(), viewport.height.toFloat(), intrinsic.first, intrinsic.second) + // Deliberately a function called from inside the gesture callbacks, not a value computed + // out here: see imageZoomTransform's comment -- the pointerInput blocks below are created + // once and keep the lambda they were created with, so anything computed at composition + // time would still be reading an empty viewport long after the image had been laid out. + fun viewportSize() = viewport.width.toFloat() to viewport.height.toFloat() Box( modifier = Modifier @@ -212,18 +157,14 @@ fun ImageZoomDialog(imageUrl: String, onDismiss: () -> Unit) { .onSizeChanged { viewport = it } .pointerInput(Unit) { detectTapGestures( - onDoubleTap = { - scale = imageZoomDoubleTapScale(scale) - offsetX = clampImagePanOffset(offsetX, viewport.width.toFloat(), fitted.first, scale) - offsetY = clampImagePanOffset(offsetY, viewport.height.toFloat(), fitted.second, scale) - } + onDoubleTap = { zoom = imageZoomDoubleTap(zoom, viewportSize(), intrinsic) } ) } .pointerInput(Unit) { val dismissDistancePx = SWIPE_DISMISS_DISTANCE.toPx() - detectTransformGestures { _, pan, zoom, _ -> - val newScale = imageZoomScale(scale, zoom) - if (newScale > 1f) { + detectTransformGestures { _, pan, gestureZoom, _ -> + val next = imageZoomTransform(zoom, viewportSize(), intrinsic, pan.x to pan.y, gestureZoom) + if (next.scale > 1f) { // Zoomed in: a drag is a pan, and can't also be a dismiss -- otherwise // panning down to the bottom of a tall image would close the viewer. dismissDragY = 0f @@ -231,9 +172,7 @@ fun ImageZoomDialog(imageUrl: String, onDismiss: () -> Unit) { dismissDragY += pan.y if (dismissDragY > dismissDistancePx) onDismiss() } - scale = newScale - offsetX = clampImagePanOffset(offsetX + pan.x, viewport.width.toFloat(), fitted.first, newScale) - offsetY = clampImagePanOffset(offsetY + pan.y, viewport.height.toFloat(), fitted.second, newScale) + zoom = next } } ) { @@ -245,10 +184,10 @@ fun ImageZoomDialog(imageUrl: String, onDismiss: () -> Unit) { modifier = Modifier .fillMaxSize() .graphicsLayer { - scaleX = scale - scaleY = scale - translationX = offsetX - translationY = offsetY + scaleX = zoom.scale + scaleY = zoom.scale + translationX = zoom.offsetX + translationY = zoom.offsetY } ) IconButton(onClick = onDismiss, modifier = Modifier.align(Alignment.TopEnd).padding(8.dp)) { diff --git a/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt b/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt new file mode 100644 index 0000000..b83b600 --- /dev/null +++ b/app/src/main/java/org/terst/feedreader/ui/ImageZoomLinks.kt @@ -0,0 +1,106 @@ +package org.terst.feedreader.ui + +// How a tap on an article image is noticed at all, and turned into a URL the viewer +// (ImageZoom.kt) can open. Split from the viewer itself because this half runs at +// HTML-assembly time, inside wrapReaderHtml -- it's reader-markup plumbing, not gestures. +// +// Article images live inside the reader's WebView, which runs with JavaScript off (see +// ReaderWebView) -- so there's no in-page script that could notice a tap on an <img> and +// report it out. And WebView's own built-in zoom (setSupportZoom/builtInZoomControls) isn't +// an option either: pinching in the reader is already taken, deliberately, by the +// reader-font-size gesture, and page zoom would scale the whole article's layout rather than +// giving an image the full screen. +// +// So tap detection is done the one way that works with JS off: every image the reader renders +// is wrapped in a link to a scheme nothing can actually navigate to, which WebView then hands +// to shouldOverrideUrlLoading -- the same interception point that already routes ordinary +// links and YouTube thumbnails. The callback opens ImageZoomDialog instead of following it. +private const val IMAGE_ZOOM_SCHEME = "feedreader-image:" + +// Matches an opening <a>, a closing </a>, or an <img> -- in document order, so a single pass +// can tell which images are already inside a link. `<a\b` deliberately doesn't match <abbr>, +// <article> etc., since \b requires a non-word character right after the "a". +private val anchorOrImageTagRegex = Regex("""<a\b[^>]*>|</a\s*>|<img\b[^>]*>""", RegexOption.IGNORE_CASE) + +private val imgSrcRegex = Regex("""\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)')""", RegexOption.IGNORE_CASE) + +private val imageFileExtensions = listOf(".jpg", ".jpeg", ".png", ".gif", ".webp", ".avif", ".bmp", ".heic", ".heif") + +/** + * Wraps every article image that isn't already inside a link in a zoom link, so tapping it + * reaches [zoomableImageTarget] via the reader WebView's shouldOverrideUrlLoading. + * + * Images that are already inside a link are left exactly as they were -- their link is doing + * something already (ReaderExtractor's synthesized YouTube thumbnail is precisely this shape, + * and wrapping it would swallow the tap that plays the video). Those are still zoomable when + * the link points at an image file, which [zoomableImageTarget] handles on the other side. + */ +fun linkifyImagesForZoom(html: String): String = buildString { + var copiedUpTo = 0 + var anchorDepth = 0 + for (tag in anchorOrImageTagRegex.findAll(html)) { + val text = tag.value + when { + text.startsWith("</", ignoreCase = true) -> if (anchorDepth > 0) anchorDepth-- + text.startsWith("<a", ignoreCase = true) -> anchorDepth++ + anchorDepth == 0 -> { + val src = imgSrcRegex.find(text)?.let { it.groupValues[1].ifEmpty { it.groupValues[2] } } + val href = src?.let { zoomHrefForSrc(it) } + if (href != null) { + append(html, copiedUpTo, tag.range.first) + append("<a href=\"").append(href).append("\">").append(text).append("</a>") + copiedUpTo = tag.range.last + 1 + } + } + } + } + append(html, copiedUpTo, html.length) +} + +private fun zoomHrefForSrc(src: String): String? { + val trimmed = src.trim() + val absolute = when { + // Protocol-relative (//host/path) is common in feed markup and resolves fine in the + // page itself, but our scheme-prefixed href has to carry a complete URL on its own. + trimmed.startsWith("//") -> "https:$trimmed" + trimmed.startsWith("http://", ignoreCase = true) || trimmed.startsWith("https://", ignoreCase = true) -> trimmed + // Anything else (site-relative, data:, javascript:) is skipped rather than guessed at: + // a relative src resolves against the WebView's base URL, which a custom-scheme href + // can't reproduce, so wrapping it would only build a dead link. + else -> null + } ?: return null + // The src came out of a quoted attribute and goes straight back into one, already escaped + // as the source wrote it. A raw quote or angle bracket could only arrive via a + // single-quoted attribute, and would break out of the href -- drop those instead. + return if (absolute.any { it == '"' || it == '<' || it == '>' }) null else IMAGE_ZOOM_SCHEME + absolute +} + +/** + * The image a tapped URL should open full-screen, or null if it isn't an image at all. + * + * Covers both halves of the problem: links [linkifyImagesForZoom] built, and links the article + * itself already had that point straight at an image file (WordPress's default "link to media + * file" wraps article images in exactly that, which used to send a tap off to LinkReaderScreen + * to try and run Readability over a JPEG). + */ +fun zoomableImageTarget(url: String): String? = when { + url.startsWith(IMAGE_ZOOM_SCHEME, ignoreCase = true) -> + url.substring(IMAGE_ZOOM_SCHEME.length).takeIf { it.isNotBlank() } + looksLikeImageUrl(url) -> url + else -> null +} + +// Extension-sniffing, not a content-type check -- there's no response to inspect at the point +// a link is tapped. SVG is deliberately not in the list: Coil's default decoders can't render +// one, so a viewer opened on it would just show an empty black screen. +private fun looksLikeImageUrl(url: String): Boolean { + val path = url.substringBefore('#').substringBefore('?').lowercase() + return imageFileExtensions.any { path.endsWith(it) } +} + +/** + * The URL to show the user for a link -- the zoom scheme is an internal detail, so a + * long-press on a zoom-wrapped image offers actions on the real image URL. + */ +fun withoutImageZoomScheme(url: String): String = + if (url.startsWith(IMAGE_ZOOM_SCHEME, ignoreCase = true)) url.substring(IMAGE_ZOOM_SCHEME.length) else url diff --git a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt index b8956f2..4ed0722 100644 --- a/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/LinkReaderScreen.kt @@ -31,10 +31,12 @@ import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -45,6 +47,8 @@ import androidx.compose.ui.viewinterop.AndroidView import kotlinx.coroutines.launch import org.terst.feedreader.data.AppDatabase import org.terst.feedreader.data.ArticleEntity +import org.terst.feedreader.data.DEFAULT_READER_FONT_SIZE_PX +import org.terst.feedreader.data.Settings import org.terst.feedreader.data.ensureStandaloneArticle import org.terst.feedreader.data.saveArticleToStash import org.terst.feedreader.data.urlToArticleId @@ -58,7 +62,9 @@ import java.net.URLEncoder * dropping into the system browser. Brought to parity with the main article reader: * save locally / save to Stash (lazily creates a standalone ArticleEntity, feedId=null, * keyed by a hash of the URL -- same scheme ShareReceiverActivity uses, so sharing the - * same link later lands on the same row instead of duplicating it), a reader-mode + * same link later lands on the same row instead of duplicating it), the saved reader font + * size and its pinch-to-resize gesture (a link read here is the same reading surface as an + * article, so it shouldn't ignore a size the article view respects), a reader-mode * toggle (falls back to the live page, JS on, when the user wants the original instead * of the extracted view or extraction failed), and the same login/search-alternate menu. */ @@ -68,9 +74,11 @@ import java.net.URLEncoder fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Unit, onOpenBrowser: (String) -> Unit) { val context = LocalContext.current val db = remember { AppDatabase.get(context) } + val settings = remember { Settings(context) } val scope = rememberCoroutineScope() val snackbarHostState = remember { SnackbarHostState() } val articleId = remember(url) { urlToArticleId(url) } + val fontSizePx by settings.readerFontSizePx.collectAsState(initial = DEFAULT_READER_FONT_SIZE_PX) var article by remember(url) { mutableStateOf<ArticleEntity?>(null) } var extractedTitle by remember(url) { mutableStateOf<String?>(null) } @@ -229,7 +237,12 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un val currentExtractedContent = extractedContent when { isReaderMode && currentExtractedContent != null -> { - val loadedContent = remember(url) { mutableStateOf<String?>(null) } + // Keyed on font size as well as content, exactly like ReaderScreen: a link + // opened in reader view is the same reading surface as an article, so it + // honors the same saved size and the same pinch-to-resize gesture, and picks + // up a size changed elsewhere while this screen stayed open. + val loadedKey = remember(url) { mutableStateOf<Pair<String?, Int>?>(null) } + val latestFontSizePx by rememberUpdatedState(fontSizePx) AndroidView( modifier = Modifier.fillMaxSize(), factory = { ctx -> @@ -239,17 +252,21 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un onOpenLink = onOpenLink, onLongPressLink = { longPressedLink = it }, onYouTubeVideo = { youtubeVideoId = it }, - onImageTap = { zoomedImageUrl = it } + onImageTap = { zoomedImageUrl = it }, + currentFontSizePx = { latestFontSizePx }, + onFontSizeChanged = { newSizePx -> scope.launch { settings.setReaderFontSizePx(newSizePx) } } ) ).apply { - loadDataWithBaseURL(url, wrapReaderHtml(currentExtractedContent, extractedTitle), "text/html", "UTF-8", null) - loadedContent.value = currentExtractedContent + val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx) + loadReaderHtml(url, html, fontSizePx) + loadedKey.value = currentExtractedContent to fontSizePx } }, update = { webView -> - if (loadedContent.value != currentExtractedContent) { - webView.loadDataWithBaseURL(url, wrapReaderHtml(currentExtractedContent, extractedTitle), "text/html", "UTF-8", null) - loadedContent.value = currentExtractedContent + if (loadedKey.value != currentExtractedContent to fontSizePx) { + val html = wrapReaderHtml(currentExtractedContent, extractedTitle, fontSizePx = fontSizePx) + webView.loadReaderHtml(url, html, fontSizePx) + loadedKey.value = currentExtractedContent to fontSizePx } }, onRelease = { webView -> webView.destroy() } @@ -276,8 +293,13 @@ fun LinkReaderScreen(url: String, onBack: () -> Unit, onOpenLink: (String) -> Un modifier = Modifier.fillMaxSize(), factory = { ctx -> WebView(ctx).apply { - settings.javaScriptEnabled = true - settings.domStorageEnabled = true + // Qualified: `settings` unqualified would now reach this + // screen's app Settings (reader font size), which shadows + // the WebView's own WebSettings inside this apply block. + // JS on here is this screen's documented fallback -- see the + // @SuppressLint and the http(s)-only guard just above. + this.settings.javaScriptEnabled = true + this.settings.domStorageEnabled = true loadUrl(url) } }, diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt index c701a26..6240b77 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -594,14 +594,14 @@ private fun ReaderContent( ) ).apply { val html = wrapReaderHtml(content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx) - loadDataWithBaseURL(baseUrl, html, "text/html", "UTF-8", null) + loadReaderHtml(baseUrl, html, fontSizePx) loadedKey.value = content to fontSizePx } }, update = { webView -> if (loadedKey.value != content to fontSizePx) { val html = wrapReaderHtml(content, article?.title, article?.publishedAt, article?.imageUrl, fontSizePx) - webView.loadDataWithBaseURL(baseUrl, html, "text/html", "UTF-8", null) + webView.loadReaderHtml(baseUrl, html, fontSizePx) loadedKey.value = content to fontSizePx } }, diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt index 9a11dd3..6d96e79 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderWebView.kt @@ -24,7 +24,9 @@ import androidx.compose.ui.viewinterop.AndroidView import androidx.compose.ui.window.Dialog import androidx.compose.ui.window.DialogProperties import org.terst.feedreader.data.DEFAULT_READER_FONT_SIZE_PX +import org.terst.feedreader.data.UNZOOMED_TEXT_ZOOM_PERCENT import org.terst.feedreader.data.pinchZoomFontSizePx +import org.terst.feedreader.data.readerTextZoomPercent // In-memory only (process lifetime, not persisted to disk) -- scoped to "don't lose my // place if Next/Previous takes me away and back," not to surviving an app restart. A @@ -99,16 +101,25 @@ private fun extractYouTubeVideoId(uri: Uri): String? { * exactly as before. ScaleGestureDetector only calls back once a second pointer is down, * so a single-finger drag never triggers any of this. * - * currentFontSizePx is read fresh (not captured once) when a gesture ends, since the - * WebView this attaches to is created once but can outlive a Settings-screen-driven font - * size change made while this same article is still open. + * currentFontSizePx is read fresh (not captured once) at the START of each gesture, since + * the WebView this attaches to is created once but can outlive a Settings-screen-driven + * font size change made while this same article is still open. Everything during the + * gesture is measured against that one starting size, so a pinch is judged against where + * it began rather than against a size it's already changed. * - * During the gesture the WebView is visually scaled live (View.scaleX/scaleY) for - * immediate feedback -- cheap, and independent of the WebView's own content layout. - * At gesture end the scale resets to 1x and onFontSizeChanged reports the new, - * clamped, rounded font size for the caller to persist; ReaderScreen's existing - * content-vs-fontSizePx reload logic (see ReaderScreen.ReaderContent) then re-renders - * the actual HTML at that size. + * The text reflows live, as the fingers move: each step sets WebSettings.textZoom, which + * re-lays-out the text at the new size right away -- no reload, no JavaScript, and + * images/padding stay exactly where they are. An earlier version instead scaled the whole + * WebView (View.scaleX/scaleY) during the gesture and snapped back to 1x on release, which + * zoomed the canvas like a photo (blurring text, magnifying the page's own margins) and + * then visibly jumped when the real font size took over. Nothing is scaled now, so there's + * nothing to snap back from. + * + * At gesture end onFontSizeChanged reports the new, clamped, rounded font size for the + * caller to persist; ReaderScreen's content-vs-fontSizePx reload logic then re-renders the + * HTML at that size via loadReaderHtml, which is also what returns textZoom to 100% -- the + * live zoom stays applied until the reload it hands over to is on screen, so the two never + * both apply and there's no gap where the text flicks back to its old size. */ data class ReaderWebViewCallbacks( val onOpenLink: (String) -> Unit, @@ -121,8 +132,30 @@ data class ReaderWebViewCallbacks( val onFontSizeChanged: (Int) -> Unit = {} ) -fun newReaderWebView(context: Context, callbacks: ReaderWebViewCallbacks): WebView = - WebView(context).apply { +/** + * The reader's WebView, which knows what font size the HTML it's showing was written at. + * + * That's the whole reason it's a class rather than a plain WebView: a live pinch lifts + * WebSettings.textZoom off 100% and leaves it there until the reload at the new font size + * arrives (see the gesture notes above). Anything that loads HTML into this WebView must put + * textZoom back, or the lifted zoom multiplies the new, already-bigger CSS size -- so loading + * goes through [loadReaderHtml], which does both together and can't be forgotten the way a + * separate "and also reset the zoom" line at each call site could. + */ +class ReaderWebView(context: Context) : WebView(context) { + /** The `font-size` the currently-loaded HTML was written with (see wrapReaderHtml). */ + var renderedFontSizePx: Int = DEFAULT_READER_FONT_SIZE_PX + private set + + fun loadReaderHtml(baseUrl: String?, html: String, fontSizePx: Int) { + renderedFontSizePx = fontSizePx + settings.textZoom = UNZOOMED_TEXT_ZOOM_PERCENT + loadDataWithBaseURL(baseUrl, html, "text/html", "UTF-8", null) + } +} + +fun newReaderWebView(context: Context, callbacks: ReaderWebViewCallbacks): ReaderWebView = + ReaderWebView(context).apply { settings.javaScriptEnabled = false overScrollMode = View.OVER_SCROLL_NEVER isVerticalScrollBarEnabled = false @@ -135,27 +168,29 @@ fun newReaderWebView(context: Context, callbacks: ReaderWebViewCallbacks): WebVi setOnScrollChangeListener { _, _, scrollY, _, _ -> callbacks.onScrollChanged(scrollY) } var pinchScale = 1f + var pinchBaseFontSizePx = DEFAULT_READER_FONT_SIZE_PX val webView = this val scaleDetector = ScaleGestureDetector( context, object : ScaleGestureDetector.SimpleOnScaleGestureListener() { override fun onScaleBegin(detector: ScaleGestureDetector): Boolean { pinchScale = 1f + pinchBaseFontSizePx = callbacks.currentFontSizePx() return true } override fun onScale(detector: ScaleGestureDetector): Boolean { pinchScale *= detector.scaleFactor - webView.scaleX = pinchScale - webView.scaleY = pinchScale + // Steps in whole pixels of font size, not in raw gesture scale, so a slow + // pinch relayouts a handful of times over its whole range rather than on + // every touch-move frame -- and lands on exactly the size it will persist. + val targetPx = pinchZoomFontSizePx(pinchBaseFontSizePx, pinchScale) + webView.settings.textZoom = readerTextZoomPercent(webView.renderedFontSizePx, targetPx) return true } override fun onScaleEnd(detector: ScaleGestureDetector) { - val newSizePx = pinchZoomFontSizePx(callbacks.currentFontSizePx(), pinchScale) - webView.scaleX = 1f - webView.scaleY = 1f - callbacks.onFontSizeChanged(newSizePx) + callbacks.onFontSizeChanged(pinchZoomFontSizePx(pinchBaseFontSizePx, pinchScale)) } } ) diff --git a/app/src/test/java/org/terst/feedreader/data/SettingsTest.kt b/app/src/test/java/org/terst/feedreader/data/SettingsTest.kt index 5b2048b..5cace1a 100644 --- a/app/src/test/java/org/terst/feedreader/data/SettingsTest.kt +++ b/app/src/test/java/org/terst/feedreader/data/SettingsTest.kt @@ -29,4 +29,27 @@ class SettingsTest { fun `pinch zoom result is clamped to the maximum reader font size`() { assertEquals(READER_FONT_SIZE_RANGE_PX.last, pinchZoomFontSizePx(28, 3.0f)) } + + @Test + fun `text already laid out at the wanted size needs no zoom`() { + assertEquals(UNZOOMED_TEXT_ZOOM_PERCENT, readerTextZoomPercent(renderedPx = 18, targetPx = 18)) + } + + @Test + fun `a bigger wanted size lifts the text zoom proportionally`() { + assertEquals(156, readerTextZoomPercent(renderedPx = 18, targetPx = 28)) + } + + @Test + fun `a smaller wanted size drops the text zoom proportionally`() { + assertEquals(78, readerTextZoomPercent(renderedPx = 18, targetPx = 14)) + } + + @Test + fun `text zoom is measured against the size the page is laid out at, not the saved size`() { + // Pinch, release, pinch again before the reload at the new size has landed: the page is + // still laid out at 18px even though 22px is the saved size, so going on to 24px has to + // be 24/18 -- measuring it against the saved 22px would visibly shrink the text first. + assertEquals(133, readerTextZoomPercent(renderedPx = 18, targetPx = 24)) + } } diff --git a/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt b/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt index 81ecc33..3f7078d 100644 --- a/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt +++ b/app/src/test/java/org/terst/feedreader/ui/ImageZoomTest.kt @@ -169,6 +169,110 @@ class ImageZoomTest { } @Test + fun `a drag on a zoomed image pans it`() { + // The regression this function exists for: the viewer computed its pan bounds from + // values captured once at composition (viewport still zero, image not yet decoded), + // which pinned every offset to 0 -- a zoomed image simply refused to pan. + val panned = imageZoomTransform( + ImageZoomState(scale = 2f), + viewportSize = 1000f to 1000f, + intrinsicSize = 1000f to 1000f, + pan = -100f to -50f, + gestureZoom = 1f + ) + assertEquals(2f, panned.scale, 0.001f) + assertEquals(-100f, panned.offsetX, 0.001f) + assertEquals(-50f, panned.offsetY, 0.001f) + } + + @Test + fun `a drag on an unzoomed image leaves it where it is`() { + val panned = imageZoomTransform( + ImageZoomState(scale = 1f), + viewportSize = 1000f to 1000f, + intrinsicSize = 1000f to 1000f, + pan = 100f to 100f, + gestureZoom = 1f + ) + assertEquals(0f, panned.offsetX, 0.001f) + assertEquals(0f, panned.offsetY, 0.001f) + } + + @Test + fun `a pan stops at the edge of the scaled image`() { + val panned = imageZoomTransform( + ImageZoomState(scale = 2f, offsetX = 400f), + viewportSize = 1000f to 1000f, + intrinsicSize = 1000f to 1000f, + pan = 300f to 0f, + gestureZoom = 1f + ) + assertEquals(500f, panned.offsetX, 0.001f) + } + + @Test + fun `a pinch zooms and clamps the existing pan to the new scale`() { + val zoomedOut = imageZoomTransform( + ImageZoomState(scale = 2f, offsetX = 500f, offsetY = 500f), + viewportSize = 1000f to 1000f, + intrinsicSize = 1000f to 1000f, + pan = 0f to 0f, + gestureZoom = 0.1f + ) + assertEquals(1f, zoomedOut.scale, 0.001f) + assertEquals(0f, zoomedOut.offsetX, 0.001f) + assertEquals(0f, zoomedOut.offsetY, 0.001f) + } + + @Test + fun `a letterboxed image pans along its long axis only`() { + // A 2000x1000 photo fitted into a 1000x1000 viewport is 1000x500 on screen, so at 2x + // it fills the viewport exactly -- there's nothing above or below it to pan to, but + // dragging sideways still works. + val panned = imageZoomTransform( + ImageZoomState(scale = 2f), + viewportSize = 1000f to 1000f, + intrinsicSize = 2000f to 1000f, + pan = -200f to 300f, + gestureZoom = 1f + ) + assertEquals(-200f, panned.offsetX, 0.001f) + assertEquals(0f, panned.offsetY, 0.001f) + } + + @Test + fun `an image that hasn't reported its size yet can still be panned`() { + // Coil reports intrinsic size only once the image decodes; a pinch-and-drag before + // then is bounded by the viewport rather than refusing to move. + val panned = imageZoomTransform( + ImageZoomState(scale = 2f), + viewportSize = 1000f to 1000f, + intrinsicSize = 0f to 0f, + pan = -300f to 0f, + gestureZoom = 1f + ) + assertEquals(-300f, panned.offsetX, 0.001f) + } + + @Test + fun `double-tapping zooms in from the fitted size`() { + val zoomed = imageZoomDoubleTap(ImageZoomState(), 1000f to 1000f, 1000f to 1000f) + assertTrue(zoomed.scale > 1f) + } + + @Test + fun `double-tapping a panned, zoomed image returns it to fitted and centered`() { + val reset = imageZoomDoubleTap( + ImageZoomState(scale = 3f, offsetX = 300f, offsetY = -200f), + 1000f to 1000f, + 1000f to 1000f + ) + assertEquals(1f, reset.scale, 0.001f) + assertEquals(0f, reset.offsetX, 0.001f) + assertEquals(0f, reset.offsetY, 0.001f) + } + + @Test fun `an image whose size isn't known yet is assumed to fill the viewport`() { // Coil only reports intrinsic size once the image has actually decoded; until then // panning has to be bounded by something, and the viewport is the safe guess. |