Skip to content

Commit 16e9344

Browse files
GarzasGarzas
authored
fix: image normalized size crash coercein [WPB-22318] (#4476)
1 parent e83121c commit 16e9344

File tree

2 files changed

+222
-8
lines changed

2 files changed

+222
-8
lines changed

‎app/src/androidTest/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/image/VisualMediaParamsTest.kt‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
/*
2+
* Wire
3+
* Copyright (C) 2024 Wire Swiss GmbH
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see http://www.gnu.org/licenses/.
17+
*/
18+
19+
package com.wire.android.ui.home.conversations.model.messagetypes.image
20+
21+
import androidx.compose.ui.test.junit4.createComposeRule
22+
import androidx.compose.ui.unit.dp
23+
import com.wire.android.ui.WireTestTheme
24+
import kotlinx.coroutines.test.runTest
25+
import org.junit.Assert.assertEquals
26+
import org.junit.Assert.assertFalse
27+
import org.junit.Assert.assertTrue
28+
import org.junit.Rule
29+
import org.junit.Test
30+
31+
class VisualMediaParamsTest {
32+
33+
@get:Rule
34+
val composeTestRule = createComposeRule()
35+
36+
@Test
37+
fun givenNonPositiveRealDimensions_shouldReturnMinSizeAndLandscapeFlag() = runTest {
38+
val params = VisualMediaParams(realMediaWidth = 0, realMediaHeight = -10)
39+
40+
val minW = 40.dp
41+
val minH = 30.dp
42+
43+
var result: NormalizedSize? = null
44+
45+
composeTestRule.setContent {
46+
WireTestTheme {
47+
result = params.normalizedSize(
48+
minW = minW,
49+
minH = minH,
50+
maxBounds = MaxBounds.DpBounds(
51+
maxW = 200.dp,
52+
maxH = 200.dp
53+
)
54+
)
55+
}
56+
}
57+
58+
composeTestRule.runOnIdle {
59+
val size = result!!
60+
assertEquals(minW, size.width)
61+
assertEquals(minH, size.height)
62+
assertFalse(size.isPortrait)
63+
}
64+
}
65+
66+
@Test
67+
fun givenLandscapeImageAndDpBounds_shouldFitWithinMaxAndRespectAspectRatio() = runTest {
68+
val params = VisualMediaParams(realMediaWidth = 1920, realMediaHeight = 1080)
69+
70+
val minW = 40.dp
71+
val minH = 40.dp
72+
val maxW = 300.dp
73+
val maxH = 300.dp
74+
75+
var result: NormalizedSize? = null
76+
77+
composeTestRule.setContent {
78+
WireTestTheme {
79+
result = params.normalizedSize(
80+
minW = minW,
81+
minH = minH,
82+
maxBounds = MaxBounds.DpBounds(
83+
maxW = maxW,
84+
maxH = maxH
85+
)
86+
)
87+
}
88+
}
89+
90+
composeTestRule.runOnIdle {
91+
val size = result!!
92+
93+
assertTrue(size.width <= maxW)
94+
assertTrue(size.height <= maxH)
95+
96+
assertTrue(size.width >= minW)
97+
assertTrue(size.height >= minH)
98+
99+
assertFalse(size.isPortrait)
100+
}
101+
}
102+
103+
@Test
104+
fun givenPortraitImageAndDpBounds_shouldFitWithinMaxAndRespectAspectRatio() = runTest {
105+
val params = VisualMediaParams(realMediaWidth = 1080, realMediaHeight = 1920)
106+
107+
val minW = 40.dp
108+
val minH = 40.dp
109+
val maxW = 300.dp
110+
val maxH = 300.dp
111+
112+
var result: NormalizedSize? = null
113+
114+
composeTestRule.setContent {
115+
WireTestTheme {
116+
result = params.normalizedSize(
117+
minW = minW,
118+
minH = minH,
119+
maxBounds = MaxBounds.DpBounds(
120+
maxW = maxW,
121+
maxH = maxH
122+
)
123+
)
124+
}
125+
}
126+
127+
composeTestRule.runOnIdle {
128+
val size = result!!
129+
130+
assertTrue(size.width <= maxW)
131+
assertTrue(size.height <= maxH)
132+
133+
assertTrue(size.width >= minW)
134+
assertTrue(size.height >= minH)
135+
136+
assertTrue(size.isPortrait)
137+
}
138+
}
139+
140+
@Test
141+
fun givenMinWidthGreaterThanMaxWidth_shouldNotCrashAndClampToMax() = runTest {
142+
val params = VisualMediaParams(realMediaWidth = 2000, realMediaHeight = 1000)
143+
144+
val minW = 300.dp
145+
val minH = 150.dp
146+
val maxW = 200.dp
147+
val maxH = 120.dp
148+
149+
var result: NormalizedSize? = null
150+
151+
composeTestRule.setContent {
152+
WireTestTheme {
153+
result = params.normalizedSize(
154+
minW = minW,
155+
minH = minH,
156+
maxBounds = MaxBounds.DpBounds(
157+
maxW = maxW,
158+
maxH = maxH
159+
)
160+
)
161+
}
162+
}
163+
164+
composeTestRule.runOnIdle {
165+
val size = result!!
166+
167+
assertEquals(maxW, size.width)
168+
169+
assertTrue(size.height > 0.dp)
170+
assertTrue(size.height <= maxH)
171+
}
172+
}
173+
174+
@Test
175+
fun givenScreenFractionBounds_shouldStayWithinCalculatedMaxBounds() = runTest {
176+
val params = VisualMediaParams(realMediaWidth = 1920, realMediaHeight = 1080)
177+
178+
val minW = 80.dp
179+
val minH = 80.dp
180+
val fractionW = 0.2f
181+
val fractionH = 0.2f
182+
183+
var result: NormalizedSize? = null
184+
185+
composeTestRule.setContent {
186+
WireTestTheme {
187+
result = params.normalizedSize(
188+
minW = minW,
189+
minH = minH,
190+
maxBounds = MaxBounds.ScreenFraction(
191+
maxWFraction = fractionW,
192+
maxHFraction = fractionH
193+
)
194+
)
195+
}
196+
}
197+
198+
composeTestRule.runOnIdle {
199+
val size = result!!
200+
201+
assertTrue(size.width > 0.dp)
202+
assertTrue(size.height > 0.dp)
203+
}
204+
}
205+
}

‎app/src/main/kotlin/com/wire/android/ui/home/conversations/model/messagetypes/image/VisualMediaParams.kt‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import androidx.compose.ui.unit.dp
2626
import androidx.compose.ui.unit.min
2727
import com.wire.android.ui.common.dimensions
2828
import kotlinx.serialization.Serializable
29-
import kotlin.let
3029

3130
/**
3231
* Parameters describing visual media (image or video) used to calculate
@@ -42,6 +41,7 @@ data class VisualMediaParams(
4241
* Returns normalized dimensions preserving the original aspect ratio.
4342
* Size is limited by [maxBounds] and coerced to at least [minW] × [minH].
4443
*/
44+
@Suppress("ReturnCount")
4545
@Composable
4646
fun normalizedSize(
4747
minW: Dp = dimensions().messageImageMinWidth,
@@ -55,7 +55,7 @@ data class VisualMediaParams(
5555
return NormalizedSize(minW, minH, isPortrait = false)
5656
}
5757

58-
val (effMaxW, effMaxH) = when (maxBounds) {
58+
val (maxWidth, maxHeight) = when (maxBounds) {
5959
is MaxBounds.DpBounds -> maxBounds.maxW to maxBounds.maxH
6060
is MaxBounds.ScreenFraction -> {
6161
fun Float.clampedFraction(): Float = coerceIn(0f, 1f)
@@ -69,15 +69,24 @@ data class VisualMediaParams(
6969
}
7070
}
7171

72+
// Guard against pathological max bounds (e.g. fraction == 0f)
73+
if (maxWidth <= 0.dp || maxHeight <= 0.dp) {
74+
return NormalizedSize(minW, minH, isPortrait = realMediaHeight > realMediaWidth)
75+
}
76+
7277
val ratio = realMediaWidth.toFloat() / realMediaHeight.toFloat()
73-
val widthFromMaxH = effMaxH * ratio
74-
val heightFromMaxW = effMaxW / ratio
7578

76-
val downW = min(effMaxW, widthFromMaxH)
77-
val downH = min(effMaxH, heightFromMaxW)
79+
val widthFromMaxH = maxHeight * ratio
80+
val heightFromMaxW = maxWidth / ratio
81+
82+
val downW = min(maxWidth, widthFromMaxH)
83+
val downH = min(maxHeight, heightFromMaxW)
84+
85+
val minAllowedW = min(minW, maxWidth)
86+
val minAllowedH = min(minH, maxHeight)
7887

79-
val finalW = downW.coerceIn(minW, effMaxW)
80-
val finalH = downH.coerceIn(minH, effMaxH)
88+
val finalW = downW.coerceIn(minAllowedW, maxWidth)
89+
val finalH = downH.coerceIn(minAllowedH, maxHeight)
8190

8291
val isPortrait = realMediaHeight > realMediaWidth
8392
return NormalizedSize(finalW, finalH, isPortrait)

0 commit comments

Comments
 (0)