diff --git a/app/src/main/java/com/nononsenseapps/feeder/model/html/HtmlLinearizer.kt b/app/src/main/java/com/nononsenseapps/feeder/model/html/HtmlLinearizer.kt index baa35912db..6d22eb65b9 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/model/html/HtmlLinearizer.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/model/html/HtmlLinearizer.kt @@ -630,8 +630,8 @@ class HtmlLinearizer { link = src, imageThumbnail = null, mimeType = source.attr("type").ifBlank { null }, - widthPx = width, - heightPx = height, + widthPx = width?.takeIf { it > 0 }, + heightPx = height?.takeIf { it > 0 }, ) } }.toList() @@ -669,8 +669,8 @@ class HtmlLinearizer { uri = video.src, link = video.link, imageThumbnail = video.imageUrl, - widthPx = width ?: video.width, - heightPx = height ?: video.height, + widthPx = width?.takeIf { it > 0 } ?: video.width.takeIf { it > 0 }, + heightPx = height?.takeIf { it > 0 } ?: video.height.takeIf { it > 0 }, mimeType = null, ), ), @@ -791,7 +791,7 @@ class HtmlLinearizer { pixelDensity = null, heightPx = null, widthPx = null, - screenWidth = width.toInt(), + screenWidth = width.toInt().takeIf { it > 0 }, ), ) } @@ -806,7 +806,7 @@ class HtmlLinearizer { result.add( LinearImageSource( imgUri = StringUtil.resolve(baseUrl, candidate.first()), - pixelDensity = density, + pixelDensity = density.takeIf { it > 0 }, heightPx = null, widthPx = null, screenWidth = null, @@ -828,8 +828,8 @@ class HtmlLinearizer { imgUri = url, pixelDensity = null, screenWidth = null, - heightPx = height, - widthPx = width, + heightPx = height.takeIf { it > 0 }, + widthPx = width.takeIf { it > 0 }, ), ) } else { @@ -853,8 +853,8 @@ class HtmlLinearizer { imgUri = url, pixelDensity = null, screenWidth = null, - heightPx = height, - widthPx = width, + heightPx = height.takeIf { it > 0 }, + widthPx = width.takeIf { it > 0 }, ), ) } else { 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 c550571de3..0771fe4e09 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 @@ -290,7 +290,22 @@ data class LinearImageSource( val heightPx: Int?, val pixelDensity: Float?, val screenWidth: Int?, -) +) { + init { + if (widthPx != null && widthPx < 1) { + throw IllegalArgumentException("Width must be positive: $widthPx") + } + if (heightPx != null && heightPx < 1) { + throw IllegalArgumentException("Height must be positive: $heightPx") + } + if (pixelDensity != null && pixelDensity <= 0) { + throw IllegalArgumentException("Pixel density must be positive: $pixelDensity") + } + if (screenWidth != null && screenWidth < 1) { + throw IllegalArgumentException("Screen width must be positive: $screenWidth") + } + } +} /** * Represents a video element @@ -318,7 +333,16 @@ data class LinearVideoSource( val widthPx: Int?, val heightPx: Int?, val mimeType: String?, -) +) { + init { + if (widthPx != null && widthPx < 1) { + throw IllegalArgumentException("Width must be positive: $widthPx") + } + if (heightPx != null && heightPx < 1) { + throw IllegalArgumentException("Height must be positive: $heightPx") + } + } +} /** * Represents an audio element 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 bc057bf9a7..feac59bc47 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 @@ -293,7 +293,13 @@ fun LinearVideoContent( else -> maxImageWidth } } - val imageHeight: Int? = null + val imageHeight: Int = + remember(linearVideo.firstSource) { + when { + linearVideo.firstSource.heightPx != null -> linearVideo.firstSource.heightPx!! + else -> imageWidth + } + } val dimens = LocalDimens.current val contentScale = @@ -311,7 +317,7 @@ fun LinearVideoContent( .data(linearVideo.imageThumbnail) .scale(Scale.FIT) // DO NOT use the actualSize parameter here - .size(Size(imageWidth, imageHeight ?: imageWidth)) + .size(Size(imageWidth, imageHeight)) // If image is larger than requested size, scale down // But if image is smaller, don't scale up // Note that this is the pixels, not how it is scaled inside the ImageView 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 63475d793e..413b9e8917 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 @@ -607,6 +607,44 @@ class HtmlLinearizerTest { ) } + @Test + fun `video with 1 source and negative width`() { + val html = "
" + val baseUrl = "https://example.com" + val result = linearizer.linearize(html, baseUrl).elements + + assertEquals(1, result.size, "Expected one item: $result") + assertEquals( + LinearVideo( + sources = + listOf( + LinearVideoSource("https://example.com/video.mp4", "https://example.com/video.mp4", null, null, null, "video/mp4"), + ), + ), + result[0], + ) + } + + @Test + fun `image with negative heightPx`() { + val html = "" + val baseUrl = "https://example.com" + val result = linearizer.linearize(html, baseUrl).elements + + assertEquals(1, result.size, "Expected one item: $result") + assertEquals( + LinearImage( + sources = + listOf( + LinearImageSource(imgUri = "https://example.com/image.jpg", widthPx = null, heightPx = null, pixelDensity = null, screenWidth = null), + ), + caption = null, + link = null, + ), + result[0], + ) + } + @Test fun `iframe with youtube video`() { val html = ""