Opened 3 years ago

Closed 2 years ago

#21712 closed defect (fixed)

Is our patch for #19212 still necessary in ESR52?

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201704R
Cc: Actual Points:
Parent ID: #20680 Points:
Reviewer: Sponsor:

Description

I reverted the patch from #19212 and saw no crash. But I need to rebuild TBB/ESR45 without the patch and try to reproduce the crash so I know I am doing the test correctly.

Child Tickets

Change History (7)

comment:1 in reply to:  description Changed 3 years ago by gk

Replying to arthuredelstein:

I reverted the patch from #19212 and saw no crash. But I need to rebuild TBB/ESR45 without the patch and try to reproduce the crash so I know I am doing the test correctly.

You don't need to rebuild it, Tor Browser 6.0 has the issue. It got fixed in 6.0.1. I just retested an old 6.0 on my machine and I still can reproduce the crash as outlined in #19212.

comment:2 Changed 2 years ago by gk

Parent ID: #20680

comment:3 Changed 2 years ago by arthuredelstein

So, like Georg, I was able to reproduce the crash using an earlier version of Tor Browser. I also could reproduce it reverting the #19212 patch in the latest TBB/ESR45.

But now I realize that in our TBB/ESR52 branch, the game shows an error message: "Failed to download game data!" This message seems to happen, I think, before the crash in the TBB/ESR45 version. So it's possible we're not actually testing the TBB/ESR52 for the crash because some other different is causing the JS to fail. So unfortunately I'm still not sure we can revert the #19212 patch.

comment:4 in reply to:  3 ; Changed 2 years ago by gk

Replying to arthuredelstein:

So, like Georg, I was able to reproduce the crash using an earlier version of Tor Browser. I also could reproduce it reverting the #19212 patch in the latest TBB/ESR45.

But now I realize that in our TBB/ESR52 branch, the game shows an error message: "Failed to download game data!" This message seems to happen, I think, before the crash in the TBB/ESR45 version.

I don't think so? comment:5:ticket:19212 seems to indicate the opposite if I am reading it right. And comment:7:ticket:19212 seems to indicate that those things are not related at all. If either of those is the case and we are reaching this message without our rebased fix and without a crash then let's get rid of it.

comment:5 Changed 2 years ago by arthuredelstein

Status: newneeds_review

OK, so in TBB/ESR52 with #19212 reverted, if I set "dom.indexeddb.enabled" to true, then the app seems to work correctly without crashing.

Whereas in TBB/ESR45 with #19212 reverted, we still get a crash. So I am confident now we can revert #19212 in TBB/ESR52.

https://github.com/arthuredelstein/tor-browser/commit/21712

comment:6 in reply to:  4 Changed 2 years ago by arthuredelstein

Keywords: TorBrowserTeam201704R added

Replying to gk:

Replying to arthuredelstein:

So, like Georg, I was able to reproduce the crash using an earlier version of Tor Browser. I also could reproduce it reverting the #19212 patch in the latest TBB/ESR45.

But now I realize that in our TBB/ESR52 branch, the game shows an error message: "Failed to download game data!" This message seems to happen, I think, before the crash in the TBB/ESR45 version.

I don't think so? comment:5:ticket:19212 seems to indicate the opposite if I am reading it right. And comment:7:ticket:19212 seems to indicate that those things are not related at all. If either of those is the case and we are reaching this message without our rebased fix and without a crash then let's get rid of it.

Yeah, oddly I don't see the "Failed to download game data!" message in TBB/ESR45. Not sure why that is. But my subsequent experiment (comment:5) is also consistent with them being separate issues.

comment:7 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks, this is ef5d0175d652406190334a77bde5c1ccf492a9a7 on tor-browser-52.0.2esr-7.0-2.

Note: See TracTickets for help on using tickets.