Conversation
…ks for existing buttons
PARAIPAN9
changed the title
Generated by 🚫 Danger Swift against e902c43 |
thatswinnie
approved these changes
Nov 20, 2024
Contributor
Author
|
@Mergifyio backport release/v133 |
Contributor
✅ Backports have been createdDetails
|
PARAIPAN9
deleted the
sparaipan/fxios-10584-20-new-homepage-tabs-are-opened-when-I-try-to-open-just-one-new-tab
branch
6 tasks
DonalMe
pushed a commit
that referenced
this pull request
Nov 20, 2024
…ned when I try to open just one new tab (backport #23253) (#23258) Bugfix FXIOS-10584 - [Toolbar Redesign] 20+ new homepage tabs are opened when I try to open just one new tab (#23253) Make self weak in the button action handler and remove code that checks for existing buttons (cherry picked from commit 2eeef55) Co-authored-by: PARAIPAN SORIN <51127880+PARAIPAN9@users.noreply.github.com>
Collaborator
|
Nice work on this @PARAIPAN9 👍 |
Contributor
Author
|
Thanks @mattreaganmozilla, I appreciate it!! |
clarmso
pushed a commit
that referenced
this pull request
Nov 25, 2024
…ned when I try to open just one new tab (#23253) Make self weak in the button action handler and remove code that checks for existing buttons
isabelrios
pushed a commit
that referenced
this pull request
Dec 3, 2024
…ned when I try to open just one new tab (#23253) Make self weak in the button action handler and remove code that checks for existing buttons
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Tickets
Jira ticket
Github issue
💡 Description
After investigating, I've found out that the bug was caused by this PR that fixed FXIOS-10500. I'm not really 100% sure what was happening but we are dealing with references here and some kind of retain cycle was certainly occuring because after tapping the
+(add new tab button) intermittently, 3, 4(a random number) of redux actions for opening a new tab were triggered.While trying to find the cause of this bug, I found an even bigger memory leak for Toolbar Buttons that were never deallocated because
selfwas captured in the action handler.Reverting the change from the PR mentioned above and fixing the leak seemed to solve both problems. FXIOS-10500 and the one addressed in this PR.
Video
video.mov
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)