Skip to content

Conversation

@SergioEstevao
Copy link
Contributor

Fixes #wordpress-mobile/WordPress-iOS#11948

This issues fixes a crash on the app when toggling a blockquote in the middle of the following HTML

<blockquote><strong>Quote One</strong></blockquote>
<blockquote><strong>Quote Two</strong></blockquote>
<blockquote><strong>Quote Three</strong></blockquote>

The crash was happening inside the method
func replaceOcurrences(of stringToFind: String, with replacementString: String, within range: NSRange) {

When I first try to unit test the original method using a vanilla NSMutableAttributedString everything was running correctly. Then I remembered that our implementation of TextStorage while it uses an NSMutableString it has special handling for the string property.
This was the reason that the range was getting invalid, after the first replacement was made and then caused a crash on the second tentative to math the occurrence.
The bug was fixed by making sure the range where we are trying to match is completely recalculated after a replacement.

To test:

  • Start the empty demo in the Calypso/Gutenberg
  • Copy paste the HTML above
  • Tap on the line with Quote Two
  • Tap on Toggle Blockquote icon on the toolbar
  • Check that app doesn't crash

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Looks good and it solves the issue! 🎉

@diegoreymendez might have something else to say though.
It looks similar to the issues we had with the last Swift migration.

Nice that it's backed with a Unit Test now.

Thank you @SergioEstevao for solving this!

@SergioEstevao SergioEstevao merged commit a006f2f into develop Jun 21, 2019
@SergioEstevao SergioEstevao deleted the issue/fix_crash_on_toogle_quotes branch June 21, 2019 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants