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

use instance var for TLD in text zoom coordinator #3823

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Jan 17, 2025

Task/Issue URL: https://app.asana.com/0/392891325557410/1209139290185318/f
Tech Design URL:
CC:

Description:
Speculative crash fix. Crashes are coming from init of TLD but there's no need for it to do that so moving to an instance variable.

Steps to test this PR:

  1. Smoke test setting the text zoom
  2. Specifically go to a website, set the zoom level. Open a new tab and visit the same site.
  3. Fire button should clear zoom levels. Visit the same site and check that it's not applied again.

@brindy brindy requested a review from miasma13 January 17, 2025 13:46
Copy link
Contributor

@miasma13 miasma13 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 , tested it and the text zoom works correctly.

One comment for potentially unrelated change.

@brindy brindy merged commit 827bc98 into main Jan 17, 2025
16 checks passed
@brindy brindy deleted the brindy/reduce-direct-init-of-tld branch January 17, 2025 14:26
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.

2 participants