Branches disappearing during review

[benjaminpjones]

Describe the bug This pops in the forums quite a bit, so I am creating a tracking issue:

When reviewing a game and using the left arrow key to navigate, MoveTree branches dissapear (Oct 2023) Bug in reviews - disappearing branches (May 2023) Variations and comments in reviews disappearing (Sep 2020)

Steps to repro

More detail in the linked forum posts, but if you use the left arrow key to navigate backwards, any branches attached to the node you are moving to get erased.

Screenshots

[GreenAsJade]

Somehow the phrase "during review" threw me off the scent here, I thought this was like the "sometimes the branches all disappear" somewhat ephemeral bug.

But this is "any time that you are looking at a review, the review information can disappear".

Fortunately, it it still there on a reload, but still it's pretty yuk.

[anoek]

I'm having trouble reproducing this, are you two seeing it?

[benjaminpjones]

Hmm.. I'm not able to reproduce either (Firefox + Mac)

[GreenAsJade]

I upgraded the priority because... yep, I see it. Chrome + Mac. I'll try to find a case...

[anoek]

Oh excellent, I've still been unable to reproduce it, glad you can

[GreenAsJade]

Hah - dunno about "reproduce". I've definitely experienced it, will go hunting.

[GreenAsJade]

Here's a public review where it happens for me - backarrow across any branch removes the branch:

https://online-go.com/review/738788

I can't find a pattern in why some reviews exhibit this behaviour and some don't.

I haven't been able to reproduce it locally.

[GreenAsJade]

Whoa fun fact: I get the problem on that review in both Chrome and Opera BUT only if I am logged in as GreenAsJade.

I don't get the problem either logged out or logged in to a different account.

Sadly, this does not help me reproduce locally. When I'm logged in as a user in the game, the review that I'm testing locally still does not do this.

[dexonsmith]

Bit of a guess, but any chance this is related to analysis being disabled? Poking around, I noticed that navPrev() has this snippet:

        if (goban.current.isAnalysisDisabled()) {
            prev.clearBranchesExceptFor(cur);
        }

Note that the review @GreenAsJade points at (https://online-go.com/review/738788) where analysis was disabled. Maybe, even though this is a review, isAnalysisDisabled() is returning the wrong answer sometimes.

Whoa fun fact: I get the problem on that review in both Chrome and Opera BUT only if I am logged in as GreenAsJade.

If my guess is right, then perhaps the bug only triggers for one of the players of the original game, which is why the bug only triggers when you're logged in as that player.

[dexonsmith]

Maybe, even though this is a review, isAnalysisDisabled() is returning the wrong answer sometimes.

Or, maybe it's consistently the same answer as the original game, but should be different?

I just posted a PR with a totally speculative fix in #2422, commit 72f9c2b70a56a55a5e056f06908a5f2564baf122. beta.online-go.com seems to be down right now so I can't test locally. I'll try to circle back in the next few days to test myself, but posting anyway in case someone else is able to test in the meantime.

[GreenAsJade]

Beta was out for a little while, I think it's back now.

[anoek]

Was this bug only reproducible on finished games? I was always testing with ongoing games, could be why I never could reproduce it.

[dexonsmith]

Confirmed that the problem disabling analysis triggers the problem with this game: https://beta.online-go.com/review/403

Even as the review controller, the branches get deleted when I press the back button. Still need to test the fix.

[dexonsmith]

Still need to test the fix.

Oh, forgot to update the issue... more details in the PR, but the fix is correct.

[pe5ha]

I have the same bug. And for me, this bug depends on which account I'm logged into. In the same browser, one account has this bug, the other does not... Please fix 🥺

[dexonsmith]

I have the same bug. And for me, this bug depends on which account I'm logged into. In the same browser, one account has this bug, the other does not... Please fix 🥺

Ah, yeah, forgot to update here; more updates in https://github.com/online-go/online-go.com/pull/2422. I did confirm the PR fixes the bug. I think it's just waiting for @anoek to merge and deploy at this point, but since this week is Thanksgiving in the US I imagine that'll wait until next week.

@pe5ha, to be sure it's covered by the patch (not an independent bug), can you confirm that:

• the account with the bug was a player in the game,
• the account without the bug was NOT a player in the game, and
• analysis was disabled for the game (or the player always has analysis disabled)?

If so, then it should be fixed by the PR.

[anoek]

Fixed in #2422