fix(YouTube - ReturnYouTubeDislike): Fix dislikes not showing on Shorts (#495)

app/src/main/java/app/revanced/integrations/patches/ReturnYouTubeDislikePatch.java

@@ -1,28 +1,33 @@
 package app.revanced.integrations.patches;
 
+import static app.revanced.integrations.returnyoutubedislike.ReturnYouTubeDislike.Vote;
+
 import android.graphics.Rect;
 import android.os.Build;
-import android.text.*;
+import android.text.Editable;
+import android.text.Spannable;
+import android.text.SpannableString;
+import android.text.Spanned;
+import android.text.TextWatcher;
 import android.view.View;
 import android.widget.TextView;
+
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
 import app.revanced.integrations.patches.components.ReturnYouTubeDislikeFilterPatch;
-import app.revanced.integrations.patches.spoof.SpoofAppVersionPatch;
 import app.revanced.integrations.returnyoutubedislike.ReturnYouTubeDislike;
 import app.revanced.integrations.settings.SettingsEnum;
 import app.revanced.integrations.shared.PlayerType;
 import app.revanced.integrations.utils.LogHelper;
 import app.revanced.integrations.utils.ReVancedUtils;
 
-import java.lang.ref.WeakReference;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.atomic.AtomicReference;
-
-import static app.revanced.integrations.returnyoutubedislike.ReturnYouTubeDislike.Vote;
-
 /**
  * Handles all interaction of UI patch components.
  *
@@ -108,7 +113,7 @@ public class ReturnYouTubeDislikePatch {
 
     /**
      * Old UI dislikes can be set multiple times by YouTube.
-     * To prevent it from reverting changes made here, this listener overrides any future changes YouTube makes.
+     * To prevent reverting changes made here, this listener overrides any future changes YouTube makes.
      */
     private static final TextWatcher oldUiTextWatcher = new TextWatcher() {
         public void beforeTextChanged(CharSequence s, int start, int count, int after) {
@@ -141,7 +146,7 @@ public class ReturnYouTubeDislikePatch {
     /**
      * Injection point.  Called on main thread.
      *
-     * Used when spoofing the older app versions of {@link SpoofAppVersionPatch}.
+     * Used when spoofing to 16.x and 17.x versions.
      */
     public static void setOldUILayoutDislikes(int buttonViewResourceId, @Nullable TextView textView) {
         try {
@@ -230,9 +235,9 @@ public class ReturnYouTubeDislikePatch {
                 }
                 ReturnYouTubeDislike videoData = lastLithoShortsVideoData;
                 if (videoData == null) {
-                    // Should not happen, as user cannot turn on RYD while leaving a short on screen.
+                    // The Shorts litho video id filter did not detect the video id.
-                    // If this does happen, then the litho video id filter did not detect the video id.
+                    // This is normal if in incognito mode, but otherwise is not normal.
-                    LogHelper.printDebug(() -> "Error: Litho video data is null, but it should not be");
+                    LogHelper.printDebug(() -> "Cannot modify Shorts litho span, data is null");
                     return original;
                 }
                 // Use the correct dislikes data after voting.
@@ -425,14 +430,18 @@ public class ReturnYouTubeDislikePatch {
      * Called both on and off main thread.
      *
      * @param isShortsLithoVideoId If the video id is from {@link ReturnYouTubeDislikeFilterPatch}.
+     *                             if true, then the video id can be null indicating the filter did
+     *                             not find any video id.
      */
-    public static void newVideoLoaded(@NonNull String videoId, boolean isShortsLithoVideoId) {
+    public static void newVideoLoaded(@Nullable String videoId, boolean isShortsLithoVideoId) {
         try {
             if (!SettingsEnum.RYD_ENABLED.getBoolean()) return;
 
             PlayerType currentPlayerType = PlayerType.getCurrent();
-            final boolean isNoneOrHidden = currentPlayerType.isNoneOrHidden();
+            final boolean isNoneHiddenOrSlidingMinimized = currentPlayerType.isNoneHiddenOrSlidingMinimized();
-            if (isNoneOrHidden && !SettingsEnum.RYD_SHORTS.getBoolean()) {
+            if (isNoneHiddenOrSlidingMinimized && !SettingsEnum.RYD_SHORTS.getBoolean()) {
+                // Must clear here, otherwise the wrong data can be used for a minimized regular video.
+                currentVideoData = null;
                 return;
             }
 
@@ -441,24 +450,41 @@ public class ReturnYouTubeDislikePatch {
                 if (videoIdIsSame(lastLithoShortsVideoData, videoId)) {
                     return;
                 }
+                if (videoId == null) {
+                    // Litho filter did not detect the video id.  App is in incognito mode,
+                    // or the proto buffer structure was changed and the video id is no longer present.
+                    // Must clear both currently playing and last litho data otherwise the
+                    // next regular video may use the wrong data.
+                    LogHelper.printDebug(() -> "Litho filter did not find any video ids");
+                    currentVideoData = null;
+                    lastLithoShortsVideoData = null;
+                    lithoShortsShouldUseCurrentData = false;
+                    return;
+                }
                 ReturnYouTubeDislike videoData = ReturnYouTubeDislike.getFetchForVideoId(videoId);
                 videoData.setVideoIdIsShort(true);
                 lastLithoShortsVideoData = videoData;
                 lithoShortsShouldUseCurrentData = false;
             } else {
+                Objects.requireNonNull(videoId);
                 // All other playback (including non-litho Shorts).
                 if (videoIdIsSame(currentVideoData, videoId)) {
                     return;
                 }
                 currentVideoData = ReturnYouTubeDislike.getFetchForVideoId(videoId);
+                // Pre-emptively set the data to short status.
+                // Required to prevent Shorts data from being used on a minimized video in incognito mode.
+                if (isNoneHiddenOrSlidingMinimized) {
+                    currentVideoData.setVideoIdIsShort(true);
+                }
             }
 
             LogHelper.printDebug(() -> "New video id: " + videoId + " playerType: " + currentPlayerType
                     + " isShortsLithoHook: " + isShortsLithoVideoId);
 
-            if (isNoneOrHidden) {
+            // Current video id hook can be called out of order with the non litho Shorts text view hook.
-                // Current video id hook can be called out of order with the non litho Shorts text view hook.
+            // Must manually update again here.
-                // Must manually update again here.
+            if (!isShortsLithoVideoId && isNoneHiddenOrSlidingMinimized) {
                 updateOnScreenShortsTextViews(true);
             }
         } catch (Exception ex) {
@@ -466,8 +492,9 @@ public class ReturnYouTubeDislikePatch {
         }
     }
 
-    private static boolean videoIdIsSame(@Nullable  ReturnYouTubeDislike fetch, String videoId) {
+    private static boolean videoIdIsSame(@Nullable ReturnYouTubeDislike fetch, @Nullable String videoId) {
-        return fetch != null && fetch.getVideoId().equals(videoId);
+        return (fetch == null && videoId == null)
+                || (fetch != null && fetch.getVideoId().equals(videoId));
     }
 
     /**
@@ -482,11 +509,13 @@ public class ReturnYouTubeDislikePatch {
             if (!SettingsEnum.RYD_ENABLED.getBoolean()) {
                 return;
             }
-            if (!SettingsEnum.RYD_SHORTS.getBoolean() && PlayerType.getCurrent().isNoneHiddenOrMinimized()) {
+            final boolean isNoneHiddenOrMinimized = PlayerType.getCurrent().isNoneHiddenOrMinimized();
+            if (isNoneHiddenOrMinimized && !SettingsEnum.RYD_SHORTS.getBoolean()) {
                 return;
             }
             ReturnYouTubeDislike videoData = currentVideoData;
             if (videoData == null) {
+                LogHelper.printDebug(() -> "Cannot send vote, as current video data is null");
                 return; // User enabled RYD while a regular video was minimized.
             }
 
@@ -494,10 +523,13 @@ public class ReturnYouTubeDislikePatch {
                 if (v.value == vote) {
                     videoData.sendVote(v);
 
-                    if (lastLithoShortsVideoData != null) {
+                    if (isNoneHiddenOrMinimized) {
-                        lithoShortsShouldUseCurrentData = true;
+                        if (lastLithoShortsVideoData != null) {
+                            lithoShortsShouldUseCurrentData = true;
+                        }
+                        updateOldUIDislikesTextView();
                     }
-                    updateOldUIDislikesTextView();
+
                     return;
                 }
             }

app/src/main/java/app/revanced/integrations/patches/components/ReturnYouTubeDislikeFilterPatch.java

@@ -2,17 +2,72 @@ package app.revanced.integrations.patches.components;
 
 import android.os.Build;
 
+import androidx.annotation.GuardedBy;
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.RequiresApi;
 
-import java.nio.charset.StandardCharsets;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
 
 import app.revanced.integrations.patches.ReturnYouTubeDislikePatch;
+import app.revanced.integrations.patches.VideoInformation;
 import app.revanced.integrations.settings.SettingsEnum;
+import app.revanced.integrations.utils.LogHelper;
+import app.revanced.integrations.utils.TrieSearch;
 
+/**
+ * Searches for video id's in the proto buffer of Shorts dislike.
+ *
+ * Because multiple litho dislike spans are created in the background
+ * (and also anytime litho refreshes the components, which is somewhat arbitrary),
+ * that makes the value of {@link VideoInformation#getVideoId()} and {@link VideoInformation#getPlayerResponseVideoId()}
+ * unreliable to determine which video id a Shorts litho span belongs to.
+ *
+ * But the correct video id does appear in the protobuffer just before a Shorts litho span is created.
+ *
+ * Once a way to asynchronously update litho text is found, this strategy will no longer be needed.
+ */
 @RequiresApi(api = Build.VERSION_CODES.N)
 public final class ReturnYouTubeDislikeFilterPatch extends Filter {
 
+    /**
+     * Last unique video id's loaded.  Value is ignored and Map is treated as a Set.
+     * Cannot use {@link LinkedHashSet} because it's missing #removeEldestEntry().
+     */
+    @GuardedBy("itself")
+    private static final Map<String, Boolean> lastVideoIds = new LinkedHashMap<>() {
+        /**
+         * Number of video id's to keep track of for searching thru the buffer.
+         * A minimum value of 3 should be sufficient, but check a few more just in case.
+         */
+        private static final int NUMBER_OF_LAST_VIDEO_IDS_TO_TRACK = 5;
+
+        @Override
+        protected boolean removeEldestEntry(Map.Entry eldest) {
+            return size() > NUMBER_OF_LAST_VIDEO_IDS_TO_TRACK;
+        }
+    };
+
+    /**
+     * Injection point.
+     */
+    public static void newPlayerResponseVideoId(String videoId) {
+        try {
+            if (!SettingsEnum.RYD_SHORTS.getBoolean()) {
+                return;
+            }
+            synchronized (lastVideoIds) {
+                if (lastVideoIds.put(videoId, Boolean.TRUE) == null) {
+                    LogHelper.printDebug(() -> "New video id: " + videoId);
+                }
+            }
+        } catch (Exception ex) {
+            LogHelper.printException(() -> "newPlayerResponseVideoId failure", ex);
+        }
+    }
+
     private final ByteArrayFilterGroupList videoIdFilterGroup = new ByteArrayFilterGroupList();
 
     public ReturnYouTubeDislikeFilterPatch() {
@@ -33,44 +88,46 @@ public final class ReturnYouTubeDislikeFilterPatch extends Filter {
                               FilterGroupList matchedList, FilterGroup matchedGroup, int matchedIndex) {
         FilterGroup.FilterGroupResult result = videoIdFilterGroup.check(protobufBufferArray);
         if (result.isFiltered()) {
-            // The video length must be hard coded to 11, as there is additional ASCII text that
+            String matchedVideoId = findVideoId(protobufBufferArray);
-            // appears immediately after the id if the dislike button is already selected.
+            // Matched video will be null if in incognito mode.
-            final int videoIdLength = 11;
+            // Must pass a null id to correctly clear out the current video data.
-            final int subStringSearchStartIndex = result.getMatchedIndex() + result.getMatchedLength();
+            // Otherwise if a Short is opened in non-incognito, then incognito is enabled and another Short is opened,
-            String videoId = findSubString(protobufBufferArray, subStringSearchStartIndex, videoIdLength);
+            // the new incognito Short will show the old prior data.
-            if (videoId != null) {
+            ReturnYouTubeDislikePatch.newVideoLoaded(matchedVideoId, true);
-                ReturnYouTubeDislikePatch.newVideoLoaded(videoId, true);
-            }
         }
 
         return false;
     }
 
-    /**
-     * Find an exact length ASCII substring starting from a given index.
-     *
-     * Similar to the String finding code in {@link LithoFilterPatch},
-     * but refactoring it to also handle this use case became messy and overly complicated.
-     */
     @Nullable
-    private static String findSubString(byte[] buffer, int bufferStartIndex, int subStringLength) {
+    private String findVideoId(byte[] protobufBufferArray) {
-        // Valid ASCII values (ignore control characters).
+        synchronized (lastVideoIds) {
-        final int minimumAscii = 32;  // 32 = space character
+            for (String videoId : lastVideoIds.keySet()) {
-        final int maximumAscii = 126; // 127 = delete character
+                if (byteArrayContainsString(protobufBufferArray, videoId)) {
-
+                    return videoId;
-        final int bufferLength = buffer.length;
+                }
-        int start = bufferStartIndex;
-        int end = bufferStartIndex;
-        do {
-            final int value = buffer[end];
-            if (value < minimumAscii || value > maximumAscii) {
-                start = end + 1;
-            } else if (end - start == subStringLength) {
-                return new String(buffer, start, subStringLength, StandardCharsets.US_ASCII);
             }
-            end++;
+            return null;
-        } while (end < bufferLength);
+        }
-
-        return null;
     }
-}
+
+    /**
+     * This could use {@link TrieSearch}, but since the video ids are constantly changing
+     * the overhead of updating the Trie might negate the search performance gain.
+     */
+    private static boolean byteArrayContainsString(@NonNull byte[] array, @NonNull String text) {
+        for (int i = 0, lastArrayStartIndex = array.length - text.length(); i <= lastArrayStartIndex; i++) {
+            boolean found = true;
+            for (int j = 0, textLength = text.length(); j < textLength; j++) {
+                if (array[i + j] != (byte) text.charAt(j)) {
+                    found = false;
+                    break;
+                }
+            }
+            if (found) {
+                return true;
+            }
+        }
+        return false;
+    }
+}

app/src/main/java/app/revanced/integrations/returnyoutubedislike/ReturnYouTubeDislike.java

@@ -74,13 +74,13 @@ public class ReturnYouTubeDislike {
     /**
      * How long to retain successful RYD fetches.
      */
-    private static final long CACHE_TIMEOUT_SUCCESS_MILLISECONDS = 5 * 60 * 1000; // 5 Minutes
+    private static final long CACHE_TIMEOUT_SUCCESS_MILLISECONDS = 7 * 60 * 1000; // 7 Minutes
 
     /**
      * How long to retain unsuccessful RYD fetches,
      * and also the minimum time before retrying again.
      */
-    private static final long CACHE_TIMEOUT_FAILURE_MILLISECONDS = 60 * 1000; // 1 Minute
+    private static final long CACHE_TIMEOUT_FAILURE_MILLISECONDS = 2 * 60 * 1000; // 2 Minutes
 
     /**
      * Unique placeholder character, used to detect if a segmented span already has dislikes added to it.
@@ -140,14 +140,10 @@ public class ReturnYouTubeDislike {
     private final long timeFetched;
 
     /**
-     * If the video id is for a Short.
+     * If this instance was previously used for a Short.
-     * Value of TRUE indicates it was previously loaded for a Short
-     * and FALSE indicates a regular video.
-     * NULL values means short status is not yet known.
      */
-    @Nullable
     @GuardedBy("this")
-    private Boolean isShort;
+    private boolean isShort;
 
     /**
      * Optional current vote status of the UI.  Used to apply a user vote that was done on a previous video viewing.
@@ -424,7 +420,6 @@ public class ReturnYouTubeDislike {
 
     /**
      * Pre-emptively set this as a Short.
-     * Should only be used immediately after creation of this instance.
      */
     public synchronized void setVideoIdIsShort(boolean isShort) {
         this.isShort = isShort;
@@ -458,35 +453,39 @@ public class ReturnYouTubeDislike {
             }
 
             synchronized (this) {
-                if (isShort != null) {
+                if (spanIsForShort) {
-                    if (isShort != spanIsForShort) {
+                    // Cannot set this to false if span is not for a Short.
-                        // user:
+                    // When spoofing to an old version and a Short is opened while a regular video
-                        // 1, opened a video
+                    // is on screen, this instance can be loaded for the minimized regular video.
-                        // 2. opened a short (without closing the regular video)
+                    // But this Shorts data won't be displayed for that call
-                        // 3. closed the short
+                    // and when it is un-minimized it will reload again and the load will be ignored.
-                        // 4. regular video is now present, but the videoId and RYD data is still for the short
+                    isShort = true;
-                        LogHelper.printDebug(() -> "Ignoring dislike span, as data loaded was previously"
+                } else if (isShort) {
-                                + " used for a different video type.");
+                    // user:
-                        return original;
+                    // 1, opened a video
-                    }
+                    // 2. opened a short (without closing the regular video)
-                } else {
+                    // 3. closed the short
-                    isShort = spanIsForShort;
+                    // 4. regular video is now present, but the videoId and RYD data is still for the short
+                    LogHelper.printDebug(() -> "Ignoring regular video dislike span,"
+                            + " as data loaded was previously used for a Short: " + videoId);
+                    return original;
                 }
 
                 if (originalDislikeSpan != null && replacementLikeDislikeSpan != null) {
                     if (spansHaveEqualTextAndColor(original, replacementLikeDislikeSpan)) {
-                        LogHelper.printDebug(() -> "Ignoring previously created dislikes span");
+                        LogHelper.printDebug(() -> "Ignoring previously created dislikes span of data: " + videoId);
                         return original;
                     }
                     if (spansHaveEqualTextAndColor(original, originalDislikeSpan)) {
-                        LogHelper.printDebug(() -> "Replacing span with previously created dislike span");
+                        LogHelper.printDebug(() -> "Replacing span with previously created dislike span of data: " + videoId);
                         return replacementLikeDislikeSpan;
                     }
                 }
                 if (isSegmentedButton && isPreviouslyCreatedSegmentedSpan(original)) {
                     // need to recreate using original, as original has prior outdated dislike values
                     if (originalDislikeSpan == null) {
-                        LogHelper.printDebug(() -> "Cannot add dislikes - original span is null"); // should never happen
+                        // Should never happen.
+                        LogHelper.printDebug(() -> "Cannot add dislikes - original span is null. videoId: " + videoId);
                         return original;
                     }
                     original = originalDislikeSpan;
@@ -514,10 +513,10 @@ public class ReturnYouTubeDislike {
         ReVancedUtils.verifyOnMainThread();
         Objects.requireNonNull(vote);
         try {
-            if (isShort != null && isShort != PlayerType.getCurrent().isNoneOrHidden()) {
+            if (isShort != PlayerType.getCurrent().isNoneOrHidden()) {
                 // Shorts was loaded with regular video present, then Shorts was closed.
                 // and then user voted on the now visible original video.
-                // Cannot send a vote, because the loaded videoId is for the wrong video.
+                // Cannot send a vote, because this instance is for the wrong video.
                 ReVancedUtils.showToastLong(str("revanced_ryd_failure_ryd_enabled_while_playing_video_then_user_voted"));
                 return;
             }

app/src/main/java/app/revanced/integrations/settingsmenu/ReturnYouTubeDislikeSettingsFragment.java

@@ -20,6 +20,10 @@ import app.revanced.integrations.settings.SharedPrefCategory;
 
 public class ReturnYouTubeDislikeSettingsFragment extends PreferenceFragment {
 
+    private static final boolean IS_SPOOFING_TO_NON_LITHO_SHORTS_PLAYER =
+            SettingsEnum.SPOOF_APP_VERSION.getBoolean()
+                    && SettingsEnum.SPOOF_APP_VERSION_TARGET.getString().compareTo("18.33.40") <= 0;
+
     /**
      * If dislikes are shown on Shorts.
      */
@@ -74,7 +78,11 @@ public class ReturnYouTubeDislikeSettingsFragment extends PreferenceFragment {
         shortsPreference = new SwitchPreference(context);
         shortsPreference.setChecked(SettingsEnum.RYD_SHORTS.getBoolean());
         shortsPreference.setTitle(str("revanced_ryd_shorts_title"));
-        shortsPreference.setSummaryOn(str("revanced_ryd_shorts_summary_on"));
+        String shortsSummary = str("revanced_ryd_shorts_summary_on",
+                IS_SPOOFING_TO_NON_LITHO_SHORTS_PLAYER
+                        ? ""
+                        : "\n\n" + str("revanced_ryd_shorts_summary_disclaimer"));
+        shortsPreference.setSummaryOn(shortsSummary);
         shortsPreference.setSummaryOff(str("revanced_ryd_shorts_summary_off"));
         shortsPreference.setOnPreferenceChangeListener((pref, newValue) -> {
             SettingsEnum.RYD_SHORTS.saveValue(newValue);

app/src/main/java/app/revanced/integrations/shared/PlayerType.kt

@@ -17,6 +17,8 @@ enum class PlayerType {
      */
     HIDDEN,
     /**
+     * A regular video is minimized.
+     *
      * When spoofing to 16.x YouTube and watching a short with a regular video in the background,
      * the type can be this (and not [HIDDEN]).
      */
@@ -26,7 +28,9 @@ enum class PlayerType {
     WATCH_WHILE_SLIDING_MAXIMIZED_FULLSCREEN,
     WATCH_WHILE_SLIDING_MINIMIZED_MAXIMIZED,
     /**
-     * When opening a short while a regular video is minimized, the type can momentarily be this.
+     * Player is either sliding to [HIDDEN] state because a Short was opened while a regular video is on screen.
+     * OR
+     * The user has swiped a minimized player away to be closed (and no Short is being opened).
      */
     WATCH_WHILE_SLIDING_MINIMIZED_DISMISSED,
     WATCH_WHILE_SLIDING_FULLSCREEN_DISMISSED,
@@ -84,20 +88,38 @@ enum class PlayerType {
         return this == NONE || this == HIDDEN
     }
 
+    /**
+     * Check if the current player type is
+     * [NONE], [HIDDEN], [WATCH_WHILE_SLIDING_MINIMIZED_DISMISSED].
+     *
+     * Useful to check if a Short is being played or opened.
+     *
+     * Usually covers all use cases with no false positives, except if called from some hooks
+     * when spoofing to an old version this will return false even
+     * though a Short is being opened or is on screen (see [isNoneHiddenOrMinimized]).
+     *
+     * @return If nothing, a Short, or a regular video is sliding off screen to a dismissed or hidden state.
+     */
+    fun isNoneHiddenOrSlidingMinimized(): Boolean {
+        return isNoneOrHidden() || this == WATCH_WHILE_SLIDING_MINIMIZED_DISMISSED
+    }
+
     /**
      * Check if the current player type is
      * [NONE], [HIDDEN], [WATCH_WHILE_MINIMIZED], [WATCH_WHILE_SLIDING_MINIMIZED_DISMISSED].
      *
      * Useful to check if a Short is being played,
-     * although will return false positive if a regular video is opened and minimized (and no short is playing).
+     * although will return false positive if a regular video is
+     * opened and minimized (and a Short is not playing or being opened).
      *
-     * @return If nothing, a Short,
+     * Typically used to detect if a Short is playing when the player cannot be in a minimized state,
-     *         or a regular video is minimized video or sliding off screen to a dismissed or hidden state.
+     * such as the user interacting with a button or element of the player.
+     *
+     * @return If nothing, a Short, a regular video is sliding off screen to a dismissed or hidden state,
+     *         a regular video is minimized (and a new video is not being opened).
      */
     fun isNoneHiddenOrMinimized(): Boolean {
-        return this == NONE || this == HIDDEN
+        return isNoneHiddenOrSlidingMinimized() || this == WATCH_WHILE_MINIMIZED
-                || this == WATCH_WHILE_MINIMIZED
-                || this == WATCH_WHILE_SLIDING_MINIMIZED_DISMISSED
     }
 
 }