Skip to content

Fix a retain cycle between RichTextView and its delegate#21062

Merged
crazytonyli merged 1 commit intotrunkfrom
weak-delegate-violation-fixes-4
Jul 18, 2023
Merged

Fix a retain cycle between RichTextView and its delegate#21062
crazytonyli merged 1 commit intotrunkfrom
weak-delegate-violation-fixes-4

Conversation

@crazytonyli
Copy link
Copy Markdown
Contributor

@crazytonyli crazytonyli commented Jul 11, 2023

Relates to #21050.

The RichTextView is only used in NoteBlockTextTableViewCell. The strong reference dataSource and delegate causes a retain cycle between the text view and the cell (its parent view).

WordPress-iOS/WordPress/Classes/ViewRelated/Notifications/Views/NoteBlockTextTableViewCell.swift

Lines 114 to 115 in 48b8a39

textView.dataSource = self
textView.delegate = self

Regression Notes

  1. Potential unintended areas of impact
    None.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    None.

  3. What automated tests I added (or what prevented me from doing so)
    None.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist: N/A

@crazytonyli crazytonyli added this to the 22.9 milestone Jul 11, 2023
@crazytonyli crazytonyli requested review from jkmassel and mokagio July 11, 2023 22:49
@crazytonyli crazytonyli self-assigned this Jul 11, 2023
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jul 11, 2023 •
edited
Loading

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr21062-901801b
Version22.7
Bundle IDorg.wordpress.alpha
Commit901801b
App Center BuildWPiOS - One-Offs #6386
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jul 11, 2023 •
edited
Loading

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr21062-901801b
Version22.7
Bundle IDcom.jetpack.alpha
Commit901801b
App Center Buildjetpack-installable-builds #5411
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@staskus staskus mentioned this pull request Jul 14, 2023
Open
staskus
staskus approved these changes Jul 18, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@staskus staskus left a comment

Choose a reason for hiding this comment

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

LGTM

@crazytonyli crazytonyli merged commit 81807b9 into trunk Jul 18, 2023
@crazytonyli crazytonyli deleted the weak-delegate-violation-fixes-4 branch July 18, 2023 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@staskus staskus staskus approved these changes

@jkmassel jkmassel Awaiting requested review from jkmassel

@mokagio mokagio Awaiting requested review from mokagio

Assignees

@crazytonyli crazytonyli

Labels

None yet

Projects

None yet

Milestone

22.9

Development

Successfully merging this pull request may close these issues.

3 participants

@crazytonyli @wpmobilebot @staskus