From edfe0f9c3060c30e208f96d6c043ffc26feb2e2d Mon Sep 17 00:00:00 2001 From: Stypox Date: Sat, 5 Jun 2021 15:35:48 +0200 Subject: [PATCH] Fix disposables handling for text linkifier also use differently Markwon methods to convert plain text to markdown --- .../fragments/detail/DescriptionFragment.java | 26 +++-- .../fragments/detail/VideoDetailFragment.java | 4 +- .../fragments/list/search/SearchFragment.java | 11 ++- .../schabi/newpipe/util/ExtractorHelper.java | 18 ++-- .../InternalUrlsHandler.java | 52 +++++----- .../external_communication/TextLinkifier.java | 94 ++++++++++--------- 6 files changed, 101 insertions(+), 104 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/DescriptionFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/DescriptionFragment.java index fcc183dad..92a571f37 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/DescriptionFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/DescriptionFragment.java @@ -31,7 +31,7 @@ import java.util.Collections; import java.util.List; import icepick.State; -import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.disposables.CompositeDisposable; import static android.text.TextUtils.isEmpty; import static org.schabi.newpipe.extractor.stream.StreamExtractor.NO_AGE_LIMIT; @@ -41,8 +41,7 @@ public class DescriptionFragment extends BaseFragment { @State StreamInfo streamInfo = null; - @Nullable - Disposable descriptionDisposable = null; + final CompositeDisposable descriptionDisposables = new CompositeDisposable(); FragmentDescriptionBinding binding; public DescriptionFragment() { @@ -67,10 +66,8 @@ public class DescriptionFragment extends BaseFragment { @Override public void onDestroy() { + descriptionDisposables.clear(); super.onDestroy(); - if (descriptionDisposable != null) { - descriptionDisposable.dispose(); - } } @@ -133,17 +130,17 @@ public class DescriptionFragment extends BaseFragment { final Description description = streamInfo.getDescription(); switch (description.getType()) { case Description.HTML: - descriptionDisposable = TextLinkifier.createLinksFromHtmlBlock( - binding.detailDescriptionView, description.getContent(), - HtmlCompat.FROM_HTML_MODE_LEGACY, streamInfo); + TextLinkifier.createLinksFromHtmlBlock(binding.detailDescriptionView, + description.getContent(), HtmlCompat.FROM_HTML_MODE_LEGACY, streamInfo, + descriptionDisposables); break; case Description.MARKDOWN: - descriptionDisposable = TextLinkifier.createLinksFromMarkdownText( - binding.detailDescriptionView, description.getContent(), streamInfo); + TextLinkifier.createLinksFromMarkdownText(binding.detailDescriptionView, + description.getContent(), streamInfo, descriptionDisposables); break; case Description.PLAIN_TEXT: default: - descriptionDisposable = TextLinkifier.createLinksFromPlainText( - binding.detailDescriptionView, description.getContent(), streamInfo); + TextLinkifier.createLinksFromPlainText(binding.detailDescriptionView, + description.getContent(), streamInfo, descriptionDisposables); break; } } @@ -198,7 +195,8 @@ public class DescriptionFragment extends BaseFragment { }); if (linkifyContent) { - TextLinkifier.createLinksFromPlainText(itemBinding.metadataContentView, content, null); + TextLinkifier.createLinksFromPlainText(itemBinding.metadataContentView, content, null, + descriptionDisposables); } else { itemBinding.metadataContentView.setText(content); } diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index c86d9596b..fbd11283f 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1546,8 +1546,8 @@ public final class VideoDetailFragment .getDefaultResolutionIndex(activity, sortedVideoStreams); updateProgressInfo(info); initThumbnailViews(info); - disposables.add(showMetaInfoInTextView(info.getMetaInfo(), binding.detailMetaInfoTextView, - binding.detailMetaInfoSeparator)); + showMetaInfoInTextView(info.getMetaInfo(), binding.detailMetaInfoTextView, + binding.detailMetaInfoSeparator, disposables); if (player == null || player.isStopped()) { updateOverlayData(info.getName(), info.getUploaderName(), info.getThumbnailUrl()); diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java index 70fce1cb7..2810b9b76 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java @@ -278,8 +278,9 @@ public class SearchFragment extends BaseListFragment cannot be bundled without creating some containers metaInfo = new MetaInfo[result.getMetaInfo().size()]; metaInfo = result.getMetaInfo().toArray(metaInfo); - disposables.add(showMetaInfoInTextView(result.getMetaInfo(), - searchBinding.searchMetaInfoTextView, searchBinding.searchMetaInfoSeparator)); + showMetaInfoInTextView(result.getMetaInfo(), searchBinding.searchMetaInfoTextView, + searchBinding.searchMetaInfoSeparator, disposables); handleSearchSuggestion(); diff --git a/app/src/main/java/org/schabi/newpipe/util/ExtractorHelper.java b/app/src/main/java/org/schabi/newpipe/util/ExtractorHelper.java index 67e12a77f..af94e3366 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ExtractorHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ExtractorHelper.java @@ -55,7 +55,7 @@ import java.util.List; import io.reactivex.rxjava3.core.Maybe; import io.reactivex.rxjava3.core.Single; -import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.disposables.CompositeDisposable; import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; @@ -269,18 +269,19 @@ public final class ExtractorHelper { * @param metaInfos a list of meta information, can be null or empty * @param metaInfoTextView the text view in which to show the formatted HTML * @param metaInfoSeparator another view to be shown or hidden accordingly to the text view - * @return a disposable to be stored somewhere and disposed when activity/fragment is destroyed + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class */ - public static Disposable showMetaInfoInTextView(@Nullable final List metaInfos, - final TextView metaInfoTextView, - final View metaInfoSeparator) { + public static void showMetaInfoInTextView(@Nullable final List metaInfos, + final TextView metaInfoTextView, + final View metaInfoSeparator, + final CompositeDisposable disposables) { final Context context = metaInfoTextView.getContext(); if (metaInfos == null || metaInfos.isEmpty() || !PreferenceManager.getDefaultSharedPreferences(context).getBoolean( context.getString(R.string.show_meta_info_key), true)) { metaInfoTextView.setVisibility(View.GONE); metaInfoSeparator.setVisibility(View.GONE); - return Disposable.empty(); } else { final StringBuilder stringBuilder = new StringBuilder(); @@ -311,9 +312,8 @@ public final class ExtractorHelper { } metaInfoSeparator.setVisibility(View.VISIBLE); - return TextLinkifier.createLinksFromHtmlBlock(metaInfoTextView, - stringBuilder.toString(), HtmlCompat.FROM_HTML_SEPARATOR_LINE_BREAK_HEADING, - null); + TextLinkifier.createLinksFromHtmlBlock(metaInfoTextView, stringBuilder.toString(), + HtmlCompat.FROM_HTML_SEPARATOR_LINE_BREAK_HEADING, null, disposables); } } diff --git a/app/src/main/java/org/schabi/newpipe/util/external_communication/InternalUrlsHandler.java b/app/src/main/java/org/schabi/newpipe/util/external_communication/InternalUrlsHandler.java index 69c846dbc..39ec51ce4 100644 --- a/app/src/main/java/org/schabi/newpipe/util/external_communication/InternalUrlsHandler.java +++ b/app/src/main/java/org/schabi/newpipe/util/external_communication/InternalUrlsHandler.java @@ -23,8 +23,6 @@ import io.reactivex.rxjava3.core.Single; import io.reactivex.rxjava3.disposables.CompositeDisposable; import io.reactivex.rxjava3.schedulers.Schedulers; -import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; - public final class InternalUrlsHandler { private static final Pattern AMPERSAND_TIMESTAMP_PATTERN = Pattern.compile("(.*)&t=(\\d+)"); private static final Pattern HASHTAG_TIMESTAMP_PATTERN = @@ -50,7 +48,7 @@ public final class InternalUrlsHandler { disposables, final Context context, @NonNull final String url) { - return handleUrl(disposables, context, url, HASHTAG_TIMESTAMP_PATTERN); + return handleUrl(context, url, HASHTAG_TIMESTAMP_PATTERN, disposables); } /** @@ -70,7 +68,7 @@ public final class InternalUrlsHandler { disposables, final Context context, @NonNull final String url) { - return handleUrl(disposables, context, url, AMPERSAND_TIMESTAMP_PATTERN); + return handleUrl(context, url, AMPERSAND_TIMESTAMP_PATTERN, disposables); } /** @@ -80,42 +78,37 @@ public final class InternalUrlsHandler { * service URL with a timestamp, the popup player will be opened and true will be returned; * else, false will be returned. * - * @param disposables a field of the Activity/Fragment class that calls this method * @param context the context to use * @param url the URL to check if it can be handled * @param pattern the pattern to use + * @param disposables a field of the Activity/Fragment class that calls this method * @return true if the URL can be handled by NewPipe, false if it cannot */ - private static boolean handleUrl(@NonNull final CompositeDisposable disposables, - final Context context, + private static boolean handleUrl(final Context context, @NonNull final String url, - @NonNull final Pattern pattern) { - final String matchedUrl; + @NonNull final Pattern pattern, + @NonNull final CompositeDisposable disposables) { + final Matcher matcher = pattern.matcher(url); + if (!matcher.matches()) { + return false; + } + final String matchedUrl = matcher.group(1); + final int seconds = Integer.parseInt(matcher.group(2)); + final StreamingService service; final StreamingService.LinkType linkType; - final int seconds; - final Matcher matcher = pattern.matcher(url); - if (matcher.matches()) { - matchedUrl = matcher.group(1); - seconds = Integer.parseInt(matcher.group(2)); - } else { - return false; - } - - if (isNullOrEmpty(matchedUrl)) { - return false; - } try { service = NewPipe.getServiceByUrl(matchedUrl); linkType = service.getLinkTypeByUrl(matchedUrl); + if (linkType == StreamingService.LinkType.NONE) { + return false; + } } catch (final ExtractionException e) { return false; } - if (linkType == StreamingService.LinkType.NONE) { - return false; - } + if (linkType == StreamingService.LinkType.STREAM && seconds != -1) { - return playOnPopup(disposables, context, matchedUrl, service, seconds); + return playOnPopup(context, matchedUrl, service, seconds, disposables); } else { NavigationHelper.openRouterActivity(context, matchedUrl); return true; @@ -125,18 +118,19 @@ public final class InternalUrlsHandler { /** * Play a content in the floating player. * - * @param disposables a field of the Activity/Fragment class that calls this method * @param context the context to be used * @param url the URL of the content * @param service the service of the content * @param seconds the position in seconds at which the floating player will start + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class * @return true if the playback of the content has successfully started or false if not */ - public static boolean playOnPopup(@NonNull final CompositeDisposable disposables, - final Context context, + public static boolean playOnPopup(final Context context, final String url, @NonNull final StreamingService service, - final int seconds) { + final int seconds, + @NonNull final CompositeDisposable disposables) { final LinkHandlerFactory factory = service.getStreamLHFactory(); final String cleanUrl; diff --git a/app/src/main/java/org/schabi/newpipe/util/external_communication/TextLinkifier.java b/app/src/main/java/org/schabi/newpipe/util/external_communication/TextLinkifier.java index 203b68726..f4b423e41 100644 --- a/app/src/main/java/org/schabi/newpipe/util/external_communication/TextLinkifier.java +++ b/app/src/main/java/org/schabi/newpipe/util/external_communication/TextLinkifier.java @@ -25,7 +25,6 @@ import io.noties.markwon.linkify.LinkifyPlugin; import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers; import io.reactivex.rxjava3.core.Single; import io.reactivex.rxjava3.disposables.CompositeDisposable; -import io.reactivex.rxjava3.disposables.Disposable; import io.reactivex.rxjava3.schedulers.Schedulers; import static org.schabi.newpipe.util.external_communication.InternalUrlsHandler.playOnPopup; @@ -42,9 +41,9 @@ public final class TextLinkifier { /** * Create web links for contents with an HTML description. *

- * This will call - * {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, Info)} - * after having linked the URLs with {@link HtmlCompat#fromHtml(String, int)}. + * This will call {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, + * Info, CompositeDisposable)} after having linked the URLs with + * {@link HtmlCompat#fromHtml(String, int)}. * * @param textView the TextView to set the htmlBlock linked * @param htmlBlock the htmlBlock to be linked @@ -53,23 +52,24 @@ public final class TextLinkifier { * @param relatedInfo if given, handle timestamps to open the stream in the popup player at * the specific time, and hashtags to search for the term in the correct * service - * @return a disposable to be stored somewhere and disposed when activity/fragment is destroyed + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class */ - @NonNull - public static Disposable createLinksFromHtmlBlock(@NonNull final TextView textView, - final String htmlBlock, - final int htmlCompatFlag, - @Nullable final Info relatedInfo) { - return changeIntentsOfDescriptionLinks( - textView, HtmlCompat.fromHtml(htmlBlock, htmlCompatFlag), relatedInfo); + public static void createLinksFromHtmlBlock(@NonNull final TextView textView, + final String htmlBlock, + final int htmlCompatFlag, + @Nullable final Info relatedInfo, + final CompositeDisposable disposables) { + changeIntentsOfDescriptionLinks( + textView, HtmlCompat.fromHtml(htmlBlock, htmlCompatFlag), relatedInfo, disposables); } /** * Create web links for contents with a plain text description. *

- * This will call - * {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, Info)} - * after having linked the URLs with {@link TextView#setAutoLinkMask(int)} and + * This will call {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, + * Info, CompositeDisposable)} after having linked the URLs with + * {@link TextView#setAutoLinkMask(int)} and * {@link TextView#setText(CharSequence, TextView.BufferType)}. * * @param textView the TextView to set the plain text block linked @@ -77,40 +77,40 @@ public final class TextLinkifier { * @param relatedInfo if given, handle timestamps to open the stream in the popup player at * the specific time, and hashtags to search for the term in the correct * service - * @return a disposable to be stored somewhere and disposed when activity/fragment is destroyed + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class */ - @NonNull - public static Disposable createLinksFromPlainText(@NonNull final TextView textView, - final String plainTextBlock, - @Nullable final Info relatedInfo) { + public static void createLinksFromPlainText(@NonNull final TextView textView, + final String plainTextBlock, + @Nullable final Info relatedInfo, + final CompositeDisposable disposables) { textView.setAutoLinkMask(Linkify.WEB_URLS); textView.setText(plainTextBlock, TextView.BufferType.SPANNABLE); - return changeIntentsOfDescriptionLinks(textView, textView.getText(), relatedInfo); + changeIntentsOfDescriptionLinks(textView, textView.getText(), relatedInfo, disposables); } /** * Create web links for contents with a markdown description. *

- * This will call - * {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, Info)} - * after creating an {@link Markwon} object and using + * This will call {@link TextLinkifier#changeIntentsOfDescriptionLinks(TextView, CharSequence, + * Info, CompositeDisposable)} after creating an {@link Markwon} object and using * {@link Markwon#setMarkdown(TextView, String)}. * * @param textView the TextView to set the plain text block linked * @param markdownBlock the block of markdown text to be linked * @param relatedInfo if given, handle timestamps to open the stream in the popup player at * the specific time, and hashtags to search for the term in the correct - * service - * @return a disposable to be stored somewhere and disposed when activity/fragment is destroyed + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class */ - @NonNull - public static Disposable createLinksFromMarkdownText(@NonNull final TextView textView, - final String markdownBlock, - @Nullable final Info relatedInfo) { + public static void createLinksFromMarkdownText(@NonNull final TextView textView, + final String markdownBlock, + @Nullable final Info relatedInfo, + final CompositeDisposable disposables) { final Markwon markwon = Markwon.builder(textView.getContext()) .usePlugin(LinkifyPlugin.create()).build(); - markwon.setMarkdown(textView, markdownBlock); - return changeIntentsOfDescriptionLinks(textView, textView.getText(), relatedInfo); + changeIntentsOfDescriptionLinks(textView, markwon.toMarkdown(markdownBlock), relatedInfo, + disposables); } /** @@ -164,11 +164,14 @@ public final class TextLinkifier { * @param spannableDescription the SpannableStringBuilder with the text of the * content description * @param relatedInfo what to open in the popup player when timestamps are clicked + * @param disposables disposables created by the method are added here and their + * lifecycle should be handled by the calling class */ private static void addClickListenersOnTimestamps(final Context context, @NonNull final SpannableStringBuilder spannableDescription, - final Info relatedInfo) { + final Info relatedInfo, + final CompositeDisposable disposables) { final String descriptionText = spannableDescription.toString(); final Matcher timestampsMatches = TIMESTAMPS_PATTERN.matcher(descriptionText); @@ -193,8 +196,8 @@ public final class TextLinkifier { spannableDescription.setSpan(new ClickableSpan() { @Override public void onClick(@NonNull final View view) { - playOnPopup(new CompositeDisposable(), context, relatedInfo.getUrl(), - relatedInfo.getService(), seconds); + playOnPopup(context, relatedInfo.getUrl(), relatedInfo.getService(), seconds, + disposables); } }, timestampStart, timestampEnd, 0); } @@ -209,8 +212,8 @@ public final class TextLinkifier { * with {@link ShareUtils#openUrlInBrowser(Context, String, boolean)}. * This method will also add click listeners on timestamps in this description, which will play * the content in the popup player at the time indicated in the timestamp, by using - * {@link TextLinkifier#addClickListenersOnTimestamps(Context, SpannableStringBuilder, Info)} - * method and click listeners on hashtags, by using + * {@link TextLinkifier#addClickListenersOnTimestamps(Context, SpannableStringBuilder, Info, + * CompositeDisposable)} method and click listeners on hashtags, by using * {@link TextLinkifier#addClickListenersOnHashtags(Context, SpannableStringBuilder, Info)}, * which will open a search on the current service with the hashtag. *

@@ -222,13 +225,14 @@ public final class TextLinkifier { * @param relatedInfo if given, handle timestamps to open the stream in the popup player at * the specific time, and hashtags to search for the term in the correct * service - * @return a disposable to be stored somewhere and disposed when activity/fragment is destroyed + * @param disposables disposables created by the method are added here and their lifecycle + * should be handled by the calling class */ - @NonNull - private static Disposable changeIntentsOfDescriptionLinks(final TextView textView, - final CharSequence chars, - @Nullable final Info relatedInfo) { - return Single.fromCallable(() -> { + private static void changeIntentsOfDescriptionLinks(final TextView textView, + final CharSequence chars, + @Nullable final Info relatedInfo, + final CompositeDisposable disposables) { + disposables.add(Single.fromCallable(() -> { final Context context = textView.getContext(); // add custom click actions on web links @@ -254,7 +258,7 @@ public final class TextLinkifier { // add click actions on plain text timestamps only for description of contents, // unneeded for meta-info or other TextViews if (relatedInfo != null) { - addClickListenersOnTimestamps(context, textBlockLinked, relatedInfo); + addClickListenersOnTimestamps(context, textBlockLinked, relatedInfo, disposables); addClickListenersOnHashtags(context, textBlockLinked, relatedInfo); } @@ -267,7 +271,7 @@ public final class TextLinkifier { Log.e(TAG, "Unable to linkify text", throwable); // this should never happen, but if it does, just fallback to it setTextViewCharSequence(textView, chars); - }); + })); } private static void setTextViewCharSequence(@NonNull final TextView textView,