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

Issue/516 crash special comments #543

Closed
wants to merge 4 commits into from

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Nov 23, 2017

Fix

Fixes #516 by by re-setting the Editable after manipulating its spans, as commented in #516 (comment)
This PR proposes the use of a method within AztecEditor to make sure the Editable is reset.

It also fixes the case commented in #516 (comment)

Note: as a consequence of this change, I've observed the onMediaDeleted method for the DeleteMediaElementWatcher was called each time a new character/span was inserted right before the Caption, so that was also fixed in f43d5e2.

Test

on API 26/27:
CASE A:

  1. start the demo app
  2. tap on any special comment (More, Page) or HorizontalRuler icons on the Aztec toolbar
  3. verify such character is inserted at the top of the demo content without crashing.

CASE B:

  1. start the demo app
  2. tap on the very left edge of the demo content image, trying to not touch the image itself, just to position the cursor at the very beginning.
  3. enter any character on the keyboard, for example "a"
  4. verify such character is inserted at the top of the demo content without crashing.

Also verify there's no changes in behavior for other API versions.

Review

@0nko

@0nko 0nko self-assigned this Nov 24, 2017
@0nko
Copy link
Contributor

0nko commented Nov 24, 2017

There is a slight problem. When I test Case B, all the headings (the style) in the app get removed.

@mzorz
Copy link
Contributor Author

mzorz commented Nov 24, 2017

There is a slight problem. When I test Case B, all the headings (the style) in the app get removed.

Out of curiosity: have you tried this very same case B with the initial solution as proposed in #516 (comment)? Does the style go away as well?

@mzorz
Copy link
Contributor Author

mzorz commented Jan 3, 2018

I think we can close this PR as the real solution will come in the form a fix from Google as stated here #516 (comment)

@mzorz mzorz closed this Jan 3, 2018
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.

Crash when applying special comments on API 26/27
2 participants