diff --git a/.gitignore b/.gitignore index 7bccc3132..a5219720c 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,6 @@ app/release/ bin/ .vscode/ *.code-workspace + +# logs +*.log diff --git a/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt new file mode 100644 index 000000000..4ced9dbc7 --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/error/ErrorInfoCommentsTest.kt @@ -0,0 +1,59 @@ +package org.schabi.newpipe.error + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.R +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import java.io.IOException +import java.net.SocketTimeoutException + +@RunWith(AndroidJUnit4::class) +class ErrorInfoCommentsTest { + private val context: Context by lazy { ApplicationProvider.getApplicationContext() } + // Test 1: Network error on initial load (Resource.Error) + @Test + fun testInitialCommentNetworkError() { + val errorInfo = ErrorInfo( + throwable = SocketTimeoutException("Connection timeout"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + Assert.assertTrue(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + Assert.assertNull(errorInfo.recaptchaUrl) + } + + // Test 2: Network error on paging (LoadState.Error) + @Test + fun testPagingNetworkError() { + val errorInfo = ErrorInfo( + throwable = IOException("Paging failed"), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.network_error), errorInfo.getMessage(context)) + Assert.assertTrue(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + Assert.assertNull(errorInfo.recaptchaUrl) + } + + // Test 3: ReCaptcha during comments load + @Test + fun testReCaptchaDuringComments() { + val url = "https://www.google.com/recaptcha/api/fallback?k=test" + val errorInfo = ErrorInfo( + throwable = ReCaptchaException("ReCaptcha needed", url), + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + Assert.assertEquals(context.getString(R.string.recaptcha_request_toast), errorInfo.getMessage(context)) + Assert.assertEquals(url, errorInfo.recaptchaUrl) + Assert.assertFalse(errorInfo.isReportable) + Assert.assertTrue(errorInfo.isRetryable) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt index 609fbb336..45ab8daa0 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt @@ -3,7 +3,6 @@ package org.schabi.newpipe.error import android.content.Context import android.os.Parcelable import androidx.annotation.StringRes -import androidx.core.content.ContextCompat import com.google.android.exoplayer2.ExoPlaybackException import com.google.android.exoplayer2.upstream.HttpDataSource import com.google.android.exoplayer2.upstream.Loader @@ -28,6 +27,7 @@ import org.schabi.newpipe.extractor.exceptions.YoutubeMusicPremiumContentExcepti import org.schabi.newpipe.ktx.isNetworkRelated import org.schabi.newpipe.player.mediasource.FailedMediaSource import org.schabi.newpipe.player.resolver.PlaybackResolver +import org.schabi.newpipe.util.Localization import java.net.UnknownHostException /** @@ -147,13 +147,11 @@ class ErrorInfo private constructor( private vararg val formatArgs: String, ) : Parcelable { fun getString(context: Context): String { + // use Localization.compatGetString() just in case context is not AppCompatActivity return if (formatArgs.isEmpty()) { - // use ContextCompat.getString() just in case context is not AppCompatActivity - ContextCompat.getString(context, stringRes) + Localization.compatGetString(context, stringRes) } else { - // ContextCompat.getString() with formatArgs does not exist, so we just - // replicate its source code but with formatArgs - ContextCompat.getContextForLanguage(context).getString(stringRes, *formatArgs) + Localization.compatGetString(context, stringRes, *formatArgs) } } } @@ -276,6 +274,9 @@ class ErrorInfo private constructor( // we don't have an exception, so this is a manually built error, which likely // indicates that it's important and is thus reportable null -> true + // a recaptcha was detected, and the user needs to solve it, there is no use in + // letting users report it + is ReCaptchaException -> false // the service explicitly said that content is not available (e.g. age restrictions, // video deleted, etc.), there is no use in letting users report it is ContentNotAvailableException -> false diff --git a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt index 669485e66..20d67a283 100644 --- a/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt +++ b/app/src/main/java/org/schabi/newpipe/paging/CommentsSource.kt @@ -16,12 +16,14 @@ class CommentsSource(private val commentInfo: CommentInfo) : PagingSource): LoadResult { // params.key is null the first time the load() function is called, so we need to return the // first batch of already-loaded comments + if (params.key == null) { return LoadResult.Page(commentInfo.comments, null, commentInfo.nextPage) } else { val info = withContext(Dispatchers.IO) { CommentsInfo.getMoreItems(service, commentInfo.url, params.key) } + return LoadResult.Page(info.items, null, info.nextPage) } } diff --git a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt index 4815965a3..fbd506456 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt +++ b/app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserPlaybackPreparer.kt @@ -6,7 +6,6 @@ import android.os.Bundle import android.os.ResultReceiver import android.support.v4.media.session.PlaybackStateCompat import android.util.Log -import androidx.core.content.ContextCompat import androidx.core.net.toUri import com.google.android.exoplayer2.Player import com.google.android.exoplayer2.ext.mediasession.MediaSessionConnector.PlaybackPreparer @@ -29,6 +28,7 @@ import org.schabi.newpipe.player.playqueue.PlaylistPlayQueue import org.schabi.newpipe.player.playqueue.SinglePlayQueue import org.schabi.newpipe.util.ChannelTabHelper import org.schabi.newpipe.util.ExtractorHelper +import org.schabi.newpipe.util.Localization import org.schabi.newpipe.util.NavigationHelper import java.util.function.BiConsumer import java.util.function.Consumer @@ -111,7 +111,7 @@ class MediaBrowserPlaybackPreparer( //region Errors private fun onUnsupportedError() { setMediaSessionError.accept( - ContextCompat.getString(context, R.string.content_not_supported), + Localization.compatGetString(context, R.string.content_not_supported), PlaybackStateCompat.ERROR_CODE_NOT_SUPPORTED ) } diff --git a/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt b/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt index ae3520c94..1e48fef5e 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/viewmodel/SettingsViewModel.kt @@ -3,13 +3,13 @@ package org.schabi.newpipe.settings.viewmodel import android.app.Application import android.content.Context import android.content.SharedPreferences -import androidx.core.content.ContextCompat import androidx.lifecycle.AndroidViewModel import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import org.schabi.newpipe.R +import org.schabi.newpipe.util.Localization import javax.inject.Inject @HiltViewModel @@ -20,11 +20,12 @@ class SettingsViewModel @Inject constructor( private var _settingsLayoutRedesignPref: Boolean get() = preferenceManager.getBoolean( - ContextCompat.getString(getApplication(), R.string.settings_layout_redesign_key), false + Localization.compatGetString(getApplication(), R.string.settings_layout_redesign_key), + false ) set(value) { preferenceManager.edit().putBoolean( - ContextCompat.getString(getApplication(), R.string.settings_layout_redesign_key), + Localization.compatGetString(getApplication(), R.string.settings_layout_redesign_key), value ).apply() } diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt new file mode 100644 index 000000000..74de30ea5 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ErrorPanel.kt @@ -0,0 +1,103 @@ +package org.schabi.newpipe.ui.components.common + +import android.content.Intent +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalInspectionMode +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.ErrorUtil +import org.schabi.newpipe.error.ReCaptchaActivity +import org.schabi.newpipe.error.UserAction +import org.schabi.newpipe.extractor.exceptions.ReCaptchaException +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.util.external_communication.ShareUtils + +@Composable +fun ErrorPanel( + errorInfo: ErrorInfo, + modifier: Modifier = Modifier, + onRetry: (() -> Unit)? = null, +) { + val context = LocalContext.current + val isPreview = LocalInspectionMode.current + val messageText = if (isPreview) { + stringResource(R.string.error_snackbar_message) + } else { + errorInfo.getMessage(context) + } + + Column( + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalAlignment = Alignment.CenterHorizontally, + modifier = modifier, + ) { + Text( + text = messageText, + style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), + textAlign = TextAlign.Center + ) + + if (errorInfo.recaptchaUrl != null) { + ServiceColoredButton(onClick = { + // Starting ReCaptcha Challenge Activity + val intent = Intent(context, ReCaptchaActivity::class.java) + .putExtra( + ReCaptchaActivity.RECAPTCHA_URL_EXTRA, + errorInfo.recaptchaUrl + ) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + context.startActivity(intent) + }) { + Text(stringResource(R.string.recaptcha_solve).uppercase()) + } + } + + if (errorInfo.isRetryable) { + onRetry?.let { + ServiceColoredButton(onClick = it) { + Text(stringResource(R.string.retry).uppercase()) + } + } + } + + if (errorInfo.isReportable) { + ServiceColoredButton(onClick = { ErrorUtil.openActivity(context, errorInfo) }) { + Text(stringResource(R.string.error_snackbar_action).uppercase()) + } + } + + errorInfo.openInBrowserUrl?.let { url -> + ServiceColoredButton(onClick = { ShareUtils.openUrlInBrowser(context, url) }) { + Text(stringResource(R.string.open_in_browser).uppercase()) + } + } + } +} + +@Preview(showBackground = true, widthDp = 360, heightDp = 640, backgroundColor = 0xffffffff) +@Composable +fun ErrorPanelPreview() { + AppTheme { + ErrorPanel( + errorInfo = ErrorInfo( + throwable = ReCaptchaException("An error", "https://example.com"), + userAction = UserAction.REQUESTED_STREAM, + request = "Preview request", + openInBrowserUrl = "https://example.com", + ), + onRetry = {}, + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt new file mode 100644 index 000000000..8b3c41a8a --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/ui/components/common/ServiceColoredButton.kt @@ -0,0 +1,54 @@ +package org.schabi.newpipe.ui.components.common + +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.RowScope +import androidx.compose.foundation.layout.wrapContentWidth +import androidx.compose.material3.Button +import androidx.compose.material3.ButtonDefaults +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.RectangleShape +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import org.schabi.newpipe.ui.theme.AppTheme +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingMedium +import org.schabi.newpipe.ui.theme.SizeTokens.SpacingSmall + +@Composable +fun ServiceColoredButton( + onClick: () -> Unit, + modifier: Modifier = Modifier, + content: @Composable() RowScope.() -> Unit, +) { + Button( + onClick = onClick, + modifier = modifier.wrapContentWidth(), + colors = ButtonDefaults.buttonColors( + containerColor = MaterialTheme.colorScheme.error, + contentColor = MaterialTheme.colorScheme.onError + ), + contentPadding = PaddingValues(horizontal = SpacingMedium, vertical = SpacingSmall), + shape = RectangleShape, + elevation = ButtonDefaults.buttonElevation( + defaultElevation = 8.dp, + + ), + ) { + content() + } +} + +@Preview +@Composable +fun ServiceColoredButtonPreview() { + AppTheme { + ServiceColoredButton( + onClick = {}, + content = { + Text("Button") + } + ) + } +} diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt index a33ffc0ca..23f13a342 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/video/comment/CommentSection.kt @@ -1,6 +1,8 @@ package org.schabi.newpipe.ui.components.video.comment import android.content.res.Configuration +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding @@ -11,6 +13,7 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.rememberNestedScrollInteropConnection @@ -25,9 +28,12 @@ import androidx.paging.compose.collectAsLazyPagingItems import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf import org.schabi.newpipe.R +import org.schabi.newpipe.error.ErrorInfo +import org.schabi.newpipe.error.UserAction import org.schabi.newpipe.extractor.Page import org.schabi.newpipe.extractor.comments.CommentsInfoItem import org.schabi.newpipe.extractor.stream.Description +import org.schabi.newpipe.ui.components.common.ErrorPanel import org.schabi.newpipe.ui.components.common.LazyColumnThemedScrollbar import org.schabi.newpipe.ui.components.common.LoadingIndicator import org.schabi.newpipe.ui.emptystate.EmptyStateComposable @@ -74,6 +80,7 @@ private fun CommentSection( modifier = Modifier .fillMaxWidth() .heightIn(min = 128.dp) + ) } } else if (count == 0) { @@ -98,21 +105,33 @@ private fun CommentSection( ) } } - - when (comments.loadState.refresh) { + when (val refresh = comments.loadState.refresh) { is LoadState.Loading -> { item { LoadingIndicator(modifier = Modifier.padding(top = 8.dp)) } } - is LoadState.Error -> { + val errorInfo = ErrorInfo( + throwable = refresh.error, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) + item { - // TODO use error panel instead - EmptyStateComposable(EmptyStateSpec.ErrorLoadingComments) + Box( + modifier = Modifier + .fillMaxWidth() + + ) { + ErrorPanel( + errorInfo = errorInfo, + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } } } - else -> { items(comments.itemCount) { Comment(comment = comments[it]!!) {} @@ -121,16 +140,24 @@ private fun CommentSection( } } } - is Resource.Error -> { + val errorInfo = ErrorInfo( + throwable = uiState.throwable, + userAction = UserAction.REQUESTED_COMMENTS, + request = "comments" + ) item { - // TODO use error panel instead - EmptyStateComposable( - spec = EmptyStateSpec.ErrorLoadingComments, + Box( modifier = Modifier - .fillMaxWidth() - .heightIn(min = 128.dp) - ) + .fillMaxSize(), + contentAlignment = Alignment.Center + ) { + ErrorPanel( + errorInfo = errorInfo, + onRetry = { comments.retry() }, + modifier = Modifier.align(Alignment.Center) + ) + } } } } diff --git a/app/src/main/java/org/schabi/newpipe/util/Localization.java b/app/src/main/java/org/schabi/newpipe/util/Localization.java index 890981e90..f5bcc40d3 100644 --- a/app/src/main/java/org/schabi/newpipe/util/Localization.java +++ b/app/src/main/java/org/schabi/newpipe/util/Localization.java @@ -18,6 +18,7 @@ import androidx.annotation.Nullable; import androidx.annotation.PluralsRes; import androidx.annotation.StringRes; import androidx.appcompat.app.AppCompatDelegate; +import androidx.core.content.ContextCompat; import androidx.core.math.MathUtils; import androidx.core.os.LocaleListCompat; import androidx.preference.PreferenceManager; @@ -71,6 +72,46 @@ public final class Localization { private Localization() { } + /** + * Gets a string like you would normally do with {@link Context#getString}, except that when + * Context is not an AppCompatActivity the correct locale is still used. The latter step uses + * {@link ContextCompat#getString}, which might fail if the Locale system service is not + * available (e.g. inside of Compose previews). In that case this method falls back to plain old + * {@link Context#getString}. + *

This method also supports format args (see {@link #compatGetString(Context, int, + * Object...)}, unlike {@link ContextCompat#getString}.

+ * + * @param context any Android context, even the App context + * @param resId the string resource to resolve + * @return the resolved string + */ + public static String compatGetString(final Context context, @StringRes final int resId) { + try { + return ContextCompat.getString(context, resId); + } catch (final Throwable e) { + return context.getString(resId); + } + } + + /** + * @see #compatGetString(Context, int) + * @param context any Android context, even the App context + * @param resId the string resource to resolve + * @param formatArgs the formatting arguments + * @return the resolved string + */ + public static String compatGetString(final Context context, + @StringRes final int resId, + final Object... formatArgs) { + try { + // ContextCompat.getString() with formatArgs does not exist, so we just + // replicate its source code but with formatArgs + return ContextCompat.getContextForLanguage(context).getString(resId, formatArgs); + } catch (final Throwable e) { + return context.getString(resId, formatArgs); + } + } + @NonNull public static String concatenateStrings(final String... strings) { return concatenateStrings(DOT_SEPARATOR, Arrays.asList(strings)); diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java index cc260d254..0a7906b8d 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -641,10 +641,9 @@ public final class NavigationHelper { public static void openSettings(final Context context) { final Class settingsClass = PreferenceManager.getDefaultSharedPreferences(context) - .getBoolean( - ContextCompat.getString(context, R.string.settings_layout_redesign_key), - false - ) ? SettingsV2Activity.class : SettingsActivity.class; + .getBoolean(Localization.compatGetString(context, + R.string.settings_layout_redesign_key), false) + ? SettingsV2Activity.class : SettingsActivity.class; final Intent intent = new Intent(context, settingsClass); context.startActivity(intent); diff --git a/app/src/main/res/layout/fragment_video_detail.xml b/app/src/main/res/layout/fragment_video_detail.xml index 1a4711581..abf1509b1 100644 --- a/app/src/main/res/layout/fragment_video_detail.xml +++ b/app/src/main/res/layout/fragment_video_detail.xml @@ -214,7 +214,6 @@ android:layout_marginTop="@dimen/video_item_detail_error_panel_margin" android:visibility="gone" tools:visibility="gone" /> -