Skip to content
This repository was archived by the owner on Feb 4, 2025. It is now read-only.

Removes font families from the stored block settings response #2904

Merged
merged 2 commits into from
Nov 23, 2023
Merged

Removes font families from the stored block settings response #2904

merged 2 commits into from
Nov 23, 2023

Conversation

@antonis
Copy link

@antonis antonis commented Nov 22, 2023 •
edited
Loading

WordPress-Android PR: wordpress-mobile/WordPress-Android#19650

Description

This PR removes the font families from the stored payload to mitigate the TransactionTooLargeException as discussed in wordpress-mobile/WordPress-Android#9685 (comment)

To Test

Use the wordpress-mobile/WordPress-Android#19650

@antonis antonis mentioned this pull request Nov 22, 2023
Merged
12 tasks
@antonis antonis marked this pull request as ready for review November 22, 2023 16:08
@antonis antonis requested a review from mkevins November 22, 2023 16:13
@antonis antonis mentioned this pull request Nov 22, 2023
Closed
@SiobhyB SiobhyB self-requested a review November 22, 2023 21:03
SiobhyB
SiobhyB reviewed Nov 22, 2023
View reviewed changes
Copy link

@SiobhyB SiobhyB left a comment

Choose a reason for hiding this comment

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

I can confirm that, with these changes applied, I'm no longer able to reproduce the crash when uploading images. I tested on @joashrajin's test site (thanks for providing me access Joash!), where I can easily reproduce the crash on a regular build. Thanks for working on this, Antonis! 🙇‍♀️

@derekblank derekblank self-requested a review November 22, 2023 22:52
derekblank
derekblank approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@derekblank derekblank left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I was also able to confirm that I am no longer experiencing the crash with these changes applied using the same sites/scenarios I was testing in wordpress-mobile/WordPress-Android#9685 (comment). 👍

@derekblank
Copy link
Contributor

derekblank commented Nov 23, 2023

Just noting I was able to successfully complete additional testing using different themes and post sizes. I did not encounter the TTLE crash or note anything unusual with theme styling.

@antonis antonis force-pushed the fix/removeFontFamiliesFromStoredTheme branch from a9870fd to cf398d5 Compare November 23, 2023 09:37
@antonis antonis changed the base branch from trunk to release/2.55.1 November 23, 2023 09:38
@antonis antonis merged commit b61e04f into release/2.55.1 Nov 23, 2023
@antonis antonis deleted the fix/removeFontFamiliesFromStoredTheme branch November 23, 2023 10:01
@antonis antonis mentioned this pull request Nov 23, 2023
Merged
@antonis
Copy link
Author

antonis commented Nov 23, 2023

Thank you both for reviewing and testing this PR. I'll target the current beta to release this asap 🙇

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mkevins mkevins Awaiting requested review from mkevins

2 more reviewers

@SiobhyB SiobhyB SiobhyB left review comments

@derekblank derekblank derekblank approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Store] Theme [Type] bug [Type] Crash

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@antonis @derekblank @SiobhyB