Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Opening PDF crashes in Brave release and Dev #3572

Closed
ryanbr opened this issue Apr 25, 2021 · 4 comments · Fixed by #3576
Closed

Opening PDF crashes in Brave release and Dev #3572

ryanbr opened this issue Apr 25, 2021 · 4 comments · Fixed by #3576

Comments

@ryanbr
Copy link

ryanbr commented Apr 25, 2021

Description:

Opening pdf https://almanacbeer.com/s/QR-MENU-42421-2-1.pdf or https://static1.squarespace.com/static/5d20f734337c400001e2aeb9/t/60848ccf24623d77653b7087/1619299536313/QR+MENU+4.24.21+%282%29+%281%29.pdf crashes

Steps to Reproduce

  1. Open https://almanacbeer.com/s/QR-MENU-42421-2-1.pdf
  2. Wait for crash
  3. Or refresh page a few times

Actual result:
Crashes back to IOs

Expected result:
Open pdf

Reproduces how often: [Easily reproduced, Intermittent Issue]
Easily, may need a few pages refreshes

Brave Version:
Brave release (1.24) and Dev (1.25)

Device details:
Iphone 8 Plus

Website problems only:

  • did you check with Brave Shields down? No
  • did you check in Safari/Firefox (WkWebView-based browsers)? No crash in Firefox

Additional Information

@kjozwiak
Copy link
Member

Reproduced pretty easily as well using iPhone 12 running iOS 14.5. Added a stack just in case but looks like @soner-yuksel already has a PR 👌

@iccub
Copy link
Contributor

iccub commented Apr 27, 2021

We decided to fix it in after-playlist release as it does not crashes on all PDFs,

the crash happens when a website's favicon size is too large, which does not happen often,
second reason the fix requires database column update, i do not want to risk any side effects here

@kjozwiak let me know if you would prefer to prioritize it for 1.25

@kjozwiak
Copy link
Member

Agreed. We're getting pretty close to completing 1.25 so introducing anything that might introduce risk, especially editing/changing database columns is probably a good idea. Definitely should get fixed in 1.26 though.

iccub added a commit that referenced this issue May 7, 2021
Adding migration to #1514 caused this regression.
First 3572 was merged then the 1514 code, which resulted in wrong
integer value.
iccub added a commit that referenced this issue May 7, 2021
…3644)

Adding migration to #1514 caused this regression.
First 3572 was merged then the 1514 code, which resulted in wrong
integer value.
@srirambv
Copy link
Contributor

Verification passed on iPhone XR with iOS 13.5 running 1.26(21.5.25.18)

  • Verified no crash on visiting either of the pages mentioned in the issue description
  • Verified no crash loading the page on both normal and private tab

Verification passed on iPhone 7+ with iOS 14.5.1 running 1.26(21.5.25.18)

  • Verified no crash on visiting either of the pages mentioned in the issue description
  • Verified no crash loading the page on both normal and private tab

Verification passed on iPad Pro with iOS 14.5.1 running 1.26(21.5.25.18)

  • Verified no crash on visiting either of the pages mentioned in the issue description
  • Verified no crash loading the page on both normal and private tab

Note: Since the PDF's are missing only checked with the page load. More discussion on this here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.