Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix FXIOS-9483 Bugzilla 1905749 #22519

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

nbhasin2
Copy link
Contributor

📜 Tickets

Jira ticket
https://bugzilla.mozilla.org/show_bug.cgi?id=1905749

💡 Description

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • 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
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@mattreaganmozilla
Copy link
Collaborator

Initial changes LGTM, do we have clear repro steps for validating? I noticed your recent comment on the Bugzilla, looks like there still might be details we're trying to clarify with the reporter?

@mattreaganmozilla
Copy link
Collaborator

@nbhasin2 Quick follow-up: is there any update on this? LMK if you want to sync further or hand this ticket off etc.

@nbhasin2 nbhasin2 marked this pull request as ready for review October 23, 2024 23:10
@nbhasin2 nbhasin2 requested a review from a team as a code owner October 23, 2024 23:10
@nbhasin2
Copy link
Contributor Author

@mattreaganmozilla handing over to you, let me know if this fix is good enough for the time being

@mobiletest-ci-bot
Copy link

Messages
📖 Project coverage: 31.91%
📖 Edited 3 files
📖 Created 0 files

Client.app: Coverage: 30.52

File Coverage
BrowserViewController+WebViewDelegates.swift 4.6% ⚠️
WebViewNavigationHandler.swift 100.0%
LegacyTabManager.swift 38.56% ⚠️

Generated by 🚫 Danger Swift against 82f313e

@mattreaganmozilla
Copy link
Collaborator

@nbhasin2 I reviewed the Bugzilla again and compared the behavior in this branch vs main and this looks Ok to me. Based on what I can see in the ticket I think we should be good to merge this and have QA validate and/or clarify any remaining questions with the reporter if needed.

@mattreaganmozilla
Copy link
Collaborator

@nbhasin2 Just double-checking: any concerns with me merging?

@nbhasin2
Copy link
Contributor Author

nbhasin2 commented Oct 24, 2024

@nbhasin2 Just double-checking: any concerns with me merging?

No concern from my end, thanks for reviewing!!

@mattreaganmozilla mattreaganmozilla merged commit c529d9f into mozilla-mobile:main Oct 24, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants