Skip to content

Commit

Permalink
Fixed crash for some images
Browse files Browse the repository at this point in the history
  • Loading branch information
@spacecowboy
spacecowboy committed Jun 10, 2024
1 parent a42529e commit 5bdd170
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 14 deletions.
20 changes: 10 additions & 10 deletions app/src/main/java/com/nononsenseapps/feeder/model/html/HtmlLinearizer.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
),
),
Expand Down Expand Up @@ -791,7 +791,7 @@ class HtmlLinearizer {
pixelDensity = null,
heightPx = null,
widthPx = null,
screenWidth = width.toInt(),
screenWidth = width.toInt().takeIf { it > 0 },
),
)
}
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
28 changes: 26 additions & 2 deletions app/src/main/java/com/nononsenseapps/feeder/model/html/LinearStuff.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions app/src/main/java/com/nononsenseapps/feeder/ui/compose/html/LinearArticleContent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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
Expand Down
38 changes: 38 additions & 0 deletions app/src/test/java/com/nononsenseapps/feeder/model/html/HtmlLinearizerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,44 @@ class HtmlLinearizerTest {
)
}

@Test
fun `video with 1 source and negative width`() {
val html = "<html><body><video controls width=\"-1\"><source src=\"video.mp4\" type=\"video/mp4\"></video></body></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 = "<html><body><img src=\"https://example.com/image.jpg\" height=\"-1\"/></body></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 = "<html><body><iframe src=\"https://www.youtube.com/embed/cjxnVO9RpaQ\"></iframe></body></html>"
Expand Down

0 comments on commit 5bdd170

Please sign in to comment.