Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable feed click events while refresh overlay is shown #6550

Merged
merged 2 commits into from
Aug 1, 2021

Conversation

Douile
Copy link
Contributor

@Douile Douile commented Jun 22, 2021

This patch changes click handlers for feed (Whats new) so that they do
nothing while the feed is refreshing and the items being clicked are not
visible.

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Prevents the user from being able to click on feed items while the refresh overlay is shown and the items themselves are hidden.

Fixes the following issue(s)

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

This patch changes click handlers for feed (Whats new) so that they do
nothing while the feed is refreshing and the items being clicked are not
visible.
@TobiGr TobiGr added bug Issue is related to a bug feed Issue is related to the feed labels Jun 22, 2021
@Douile
Copy link
Contributor Author

Douile commented Jun 23, 2021

Im not sure why CI failed. I ran ./gradleW connectCheck locally and it was fine

@B0pol
Copy link
Member

B0pol commented Jun 24, 2021

it fails not because of your changes, see #6560. If you rebase it should be ok

@B0pol
Copy link
Member

B0pol commented Jun 24, 2021

It produces error when you cilck / long click and the feed stops loading

Exception

  • User Action: requested feed
  • Request: Loading feed
  • Content Country: FR
  • Content Language: fr-FR
  • App Language: fr_FR
  • Service: none
  • Version: 0.21.5
  • OS: Linux Android 9 - 28
Crash log

java.lang.NullPointerException: Attempt to invoke virtual method 'void org.schabi.newpipe.database.subscription.SubscriptionEntity.setData(java.lang.String, java.lang.String, java.lang.String, java.lang.Long)' on a null object reference
	at org.schabi.newpipe.local.subscription.SubscriptionManager.updateFromInfo(SubscriptionManager.kt:75)
	at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1$1.run(FeedLoadService.kt:324)
	at androidx.room.RoomDatabase.runInTransaction(RoomDatabase.java:556)
	at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:316)
	at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:65)
	at io.reactivex.rxjava3.internal.operators.flowable.FlowableDoOnEach$DoOnEachSubscriber.onNext(FlowableDoOnEach.java:86)
	at io.reactivex.rxjava3.internal.operators.flowable.FlowableBuffer$PublisherBufferExactSubscriber.onNext(FlowableBuffer.java:124)
	at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$ObserveOnSubscriber.runAsync(FlowableObserveOn.java:402)
	at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$BaseObserveOnSubscriber.run(FlowableObserveOn.java:176)
	at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:65)
	at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:56)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.lang.Thread.run(Thread.java:764)


B0pol
B0pol previously requested changes Jun 24, 2021
Copy link
Member

@B0pol B0pol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix errors

@Douile
Copy link
Contributor Author

Douile commented Jun 26, 2021

@B0pol are you sure this error is caused by this PR? Not sure if I was doing something wrong but was unable to replicate as you described. I was able to replicate the same crash (on dev branch) by:

  1. starting a feed refresh
  2. swapping to subscriptions tab
  3. Unsubscribing from someone near the bottom of list (hard to do quick enough unless you have a decent amount of subscriptions)

@B0pol
Copy link
Member

B0pol commented Jun 26, 2021

Yes I am. Or at least it is really weird if it's not caused by your PR.

  1. Using your version it doesn't fix the issues you mentionned
  2. It cause errors as I said.

Here is a video:

6550.mp4

@Douile
Copy link
Contributor Author

Douile commented Jun 26, 2021

Okay thanks. I might try to switch to getting the UI elements themselves to block anyway as this fix is rather hacky.

@Douile
Copy link
Contributor Author

Douile commented Jul 11, 2021

I am now able to replicate the bug after building on a fresh install of android-studio. I went for these easy fix of checking for null before calling methods on the object.

org.schabi.newpipe.local.subscription.SubscriptionManager.kt
fun updateFromInfo(subscriptionId: Long, info: ListInfo<StreamInfoItem>) {
        val subscriptionEntity = subscriptionTable.getSubscription(subscriptionId)

        if (subscriptionEntity == null) return

        if (info is FeedInfo) {
            subscriptionEntity.name = info.name
        } else if (info is ChannelInfo) {
            subscriptionEntity.setData(info.name, info.avatarUrl, info.description, info.subscriberCount)
        }

        subscriptionTable.update(subscriptionEntity)
    }

For some reason android studio tells me subscriptionEntity will never be null? As in fetching an entity from the database will never return no result, but as evidenced by the crash it seems it can. However compiling this code anyway you seem to run into a bunch of seemingly unrelated sqlite errors todo with transactions occurring at the same time.

Crash logs
org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:21.155 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:22.794 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:22.812 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:24.367 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:24.380 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:25.037 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:25.050 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:26.455 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:26.465 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:26.595 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:26.602 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:35.253 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:37.722 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:37.744 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:38.484 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:38.492 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/oughfeedrefres: No package ID ff found for ID 0xffffffff.
2021-07-11 02:34:40.947 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/class org.schabi.newpipe.error.ErrorActivity: android.database.sqlite.SQLiteConstraintException: FOREIGN KEY constraint failed (code 787 SQLITE_CONSTRAINT_FOREIGNKEY)
        at android.database.sqlite.SQLiteConnection.nativeExecute(Native Method)
        at android.database.sqlite.SQLiteConnection.execute(SQLiteConnection.java:707)
        at android.database.sqlite.SQLiteSession.endTransactionUnchecked(SQLiteSession.java:439)
        at android.database.sqlite.SQLiteSession.endTransaction(SQLiteSession.java:403)
        at android.database.sqlite.SQLiteDatabase.endTransaction(SQLiteDatabase.java:588)
        at androidx.sqlite.db.framework.FrameworkSQLiteDatabase.endTransaction(FrameworkSQLiteDatabase.java:90)
        at androidx.room.RoomDatabase.internalEndTransaction(RoomDatabase.java:510)
        at androidx.room.RoomDatabase.endTransaction(RoomDatabase.java:500)
        at androidx.room.RoomDatabase.runInTransaction(RoomDatabase.java:559)
        at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:316)
        at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:65)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableDoOnEach$DoOnEachSubscriber.onNext(FlowableDoOnEach.java:86)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableBuffer$PublisherBufferExactSubscriber.onNext(FlowableBuffer.java:124)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$ObserveOnSubscriber.runAsync(FlowableObserveOn.java:402)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$BaseObserveOnSubscriber.run(FlowableObserveOn.java:176)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:65)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:56)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:923)
2021-07-11 02:35:40.082 23503-23653/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/SQLiteLog: (1) statement aborts at 3: [BEGIN IMMEDIATE;] cannot start a transaction within a transaction
2021-07-11 02:35:41.699 23503-23503/org.schabi.newpipe.debug.fixclickthroughfeedrefresh E/class org.schabi.newpipe.error.ErrorActivity: android.database.sqlite.SQLiteException: cannot start a transaction within a transaction (code 1 SQLITE_ERROR)
        at android.database.sqlite.SQLiteConnection.nativeExecute(Native Method)
        at android.database.sqlite.SQLiteConnection.execute(SQLiteConnection.java:707)
        at android.database.sqlite.SQLiteSession.beginTransactionUnchecked(SQLiteSession.java:321)
        at android.database.sqlite.SQLiteSession.beginTransaction(SQLiteSession.java:300)
        at android.database.sqlite.SQLiteDatabase.beginTransaction(SQLiteDatabase.java:571)
        at android.database.sqlite.SQLiteDatabase.beginTransactionNonExclusive(SQLiteDatabase.java:505)
        at androidx.sqlite.db.framework.FrameworkSQLiteDatabase.beginTransactionNonExclusive(FrameworkSQLiteDatabase.java:74)
        at androidx.room.RoomDatabase.internalBeginTransaction(RoomDatabase.java:486)
        at androidx.room.RoomDatabase.beginTransaction(RoomDatabase.java:471)
        at androidx.room.RoomDatabase.runInTransaction(RoomDatabase.java:554)
        at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:316)
        at org.schabi.newpipe.local.feed.service.FeedLoadService$databaseConsumer$1.accept(FeedLoadService.kt:65)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableDoOnEach$DoOnEachSubscriber.onNext(FlowableDoOnEach.java:86)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableBuffer$PublisherBufferExactSubscriber.onNext(FlowableBuffer.java:124)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$ObserveOnSubscriber.runAsync(FlowableObserveOn.java:402)
        at io.reactivex.rxjava3.internal.operators.flowable.FlowableObserveOn$BaseObserveOnSubscriber.run(FlowableObserveOn.java:176)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:65)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:56)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:923)

If anyone has any insights into this stuff please let me know otherwise I might have to burn this PR: as I don't really know kotlin and am unsure how disabling click events during load is causing null pointer exceptions on a different panel.

@Douile
Copy link
Contributor Author

Douile commented Jul 11, 2021

Never mind, not being lazy and creating a variable instead of using the already existing binding seems to fix the problem. I am not going to pretend to understand how.

@B0pol would you mind confirming please.
:) thanks

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm this simple fix works. Thank you for figuring it out! (btw the base cause was this method, but it can't be removed because that would create scrolling bugs in the video detail fragment)

@Stypox Stypox merged commit a9623f8 into TeamNewPipe:dev Aug 1, 2021
This was referenced Aug 4, 2021
ShareASmile pushed a commit to ShareASmile/NewPipeZing that referenced this pull request Aug 16, 2021
…-refresh

Disable feed click events while refresh overlay is shown
ShareASmile added a commit to ShareASmile/NewPipeZing that referenced this pull request Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug feed Issue is related to the feed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subscription Feed Clickable Through Refresh Popup
4 participants