firefox-ios icon indicating copy to clipboard operation
firefox-ios copied to clipboard
verified publisher icon mozilla-mobile

Bugfix FXIOS-8416 [v124] Fix potential KVO crash if BVC is dellocated before unregistering

Open mattreaganmozilla opened this issue 1 year ago • 1 comments

:scroll: Tickets

Jira ticket Github issue

:bulb: Description

This is a fix to avoid potential crashes in WebKit if the BVC or its scrollController are deallocated without de-registering them as observers of the WKWebView or its scroll view.

(Related discussion thread: https://mozilla.slack.com/archives/C05C9RET70F/p1707832410945339 )

:pencil: Checklist

You have to check all boxes before merging

  • [x] Filled in the above information (tickets numbers and description of your work)
  • [x] Updated the PR name to follow our PR naming guidelines
  • [ ] Wrote unit tests and/or ensured the tests suite is passing
  • [ ] When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • [ ] If needed I updated documentation / comments for complex code and public methods

mattreaganmozilla avatar Feb 16 '24 00:02 mattreaganmozilla

Going to open as a draft, this is ready for review but I'd like to do one additional round of testing before merging.

mattreaganmozilla avatar Feb 16 '24 00:02 mattreaganmozilla

Messages
:book: Project coverage: 33.49%
:book: Edited 3 files
:book: Created 0 files

Client.app: Coverage: 32.28

File Coverage
BrowserViewController.swift 21.21% ⚠️
TabScrollController.swift 70.17% ✅

Generated by :no_entry_sign: Danger Swift against 80deea772284327b6d1b47e3719c6b34e205945d

mobiletest-ci-bot avatar Feb 19 '24 18:02 mobiletest-ci-bot

This was intended to target v124 (was merged 2/20) but it looks like these changes never made it into release/v124.

Because these changes are needed for my latest follow-up fix (https://github.com/mozilla-mobile/firefox-ios/pull/19211) targeting weekly, I'm planning to backport this PR to v124 branch for the weekly.

If anyone has concerns about this please LMK. Thank you cc @dsmithpadilla @OrlaM @lmarceau

mattreaganmozilla avatar Mar 14 '24 19:03 mattreaganmozilla

@mergifyio backport release/v124

mattreaganmozilla avatar Mar 14 '24 19:03 mattreaganmozilla

backport release/v124

✅ Backports have been created

mergify[bot] avatar Mar 14 '24 19:03 mergify[bot]