wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
675 stars 112 forks source link

Crash when applying special comments on API 26/27 #516

Closed 0nko closed 5 years ago

0nko commented 6 years ago

Applying a special comment (more, page) results in a crash on Oreo 8.1 (API 27)

Reproduced

  1. Start the demo app (in Calypso mode)
  2. Tap on the More button
  3. Observe the crash
    FATAL EXCEPTION: main
    Process: org.wordpress.aztec, PID: 6582
    java.lang.ArrayIndexOutOfBoundsException: length=39; index=-1
    at android.text.DynamicLayout.getBlockIndex(DynamicLayout.java:648)
    at android.widget.Editor.drawHardwareAccelerated(Editor.java:1703)
    at android.widget.Editor.onDraw(Editor.java:1672)
    at android.widget.TextView.onDraw(TextView.java:6882)
    at android.view.View.draw(View.java:19192)
    at android.view.View.updateDisplayListIfDirty(View.java:18142)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.draw(View.java:19195)
    at android.widget.ScrollView.draw(ScrollView.java:1739)
    at android.view.View.updateDisplayListIfDirty(View.java:18142)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.updateDisplayListIfDirty(View.java:18133)
    at android.view.View.draw(View.java:18920)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4236)
    at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4022)
    at android.view.View.draw(View.java:19195)
    at com.android.internal.policy.DecorView.draw(DecorView.java:788)
    at android.view.View.updateDisplayListIfDirty(View.java:18142)
    at android.view.ThreadedRenderer.updateViewTreeDisplayList(ThreadedRenderer.java:669)
    at android.view.ThreadedRenderer.updateRootDisplayList(ThreadedRenderer.java:675)
    at android.view.ThreadedRenderer.draw(ThreadedRenderer.java:783)
    at android.view.ViewRootImpl.draw(ViewRootImpl.java:2992)
    at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:2806)
    at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2359)
    at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1392)
    at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6752)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:911)
    at android.view.Choreographer.doCallbacks(Choreographer.java:723)
    at android.view.Choreographer.doFrame(Choreographer.java:658)
    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:897)
    at android.os.Handler.handleCallback(Handler.java:790)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:164)
    at android.app.ActivityThread.main(ActivityThread.java:6494)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)

Tested

Emulator running API 27.

mzorz commented 6 years ago

Found this only happens if you follow the steps above by the letter. But, if you position the cursor somewhere in the body first (by tapping before/after anything in the content area), then tapping on one of the special comments action icons works alright, inserting the desired object on the desired position.

mzorz commented 6 years ago

More info on this: this only happens if the cursor is at the very beginning of the content: tapping at the left of the first image in the Aztec demo app shows the cursor blinking, and then trying to insert something there (more, page OR horizontal ruler) makes it fail

mzorz commented 6 years ago

Confirming this happens since API 26 as well (Android 8.0.0)

mzorz commented 6 years ago

Managed to narrow this down to:

For example, in the demo app, trying to insert a Horizontal Rule right before the following content produces the crash:

"[caption]<img src=\"https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png\" />ExampleCaption[/caption]"

However, this other content does not produce the crash (it's just the same as above without the ExampleCaption text, and more specifically without any caption text, even when the [caption] tag is present):

"[caption]<img src=\"https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png\" />[/caption]"

Still trying to gather information to see what relates [caption] with how the horizontal rule / special comments are related.

mzorz commented 6 years ago

I also managed to make it crash with the very/similar output when inserting ANY character right before the captioned image, although I believe this might be a different problem (the cause certainly looks different, although the crash is the same):

  1. open the demo app
  2. tap on the very left edge of the image (try not to tap the image itself), and see the cursor appears and the keyboard is displayed
  3. tap on any character, i.e. โ€œaโ€
  4. see it crashes.

Apparently, checking the code I can avoid this crash from happening when commenting out the following piece of code, which was introduced in commit https://github.com/wordpress-mobile/AztecEditor-Android/commit/668dcfb078b924cfaeb88bbaa775311d4b9e9a49 :

https://github.com/wordpress-mobile/AztecEditor-Android/blob/develop/wordpress-shortcodes/src/main/java/org/wordpress/aztec/plugins/shortcodes/watchers/CaptionWatcher.kt#L16-L24

If you comment out those lines, following the above steps 1-4 donโ€™t make it crash. @0nko do you see anything here that might be a root/shared cause?

0nko commented 6 years ago

I've played around with the app and I can confirm what you mentioned above. Setting the HTML to the following will always crash:

[caption align="alignright"]<img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption[/caption]

Apparently, checking the code I can avoid this crash from happening when commenting out the following piece of code, which was introduced in commit 668dcfb

The code above is just one of many manifestations of the same error and it's not limited to CaptionWatcher. It appears that in specific situations setSpan() throws that exception but it doesn't in others. For the example above, adding a character before an image calls setSpan(<caption_span>, 1, span.end, span.flags). If I changed the start position to 0 it would not crash, however. Also changing the span to an inline span also works even with the original bounds.

I tried a couple of things, like removing some other spans, removing and then setting the caption span but nothing worked. The only thing that did work was setting a span on a separate Editable, like this:

val t = aztecText.text
t.setSpan(...)
aztecText.text = t

It could probably work for the other case with the special comment, we'd just need to find the specific call to setSpan().

mzorz commented 6 years ago

Nice find @0nko ! I'm surprised that val t = aztecText.text produces a different Editable (I would have thought it's the same reference). Will try for both cases and come back here ๐Ÿ‘

0nko commented 6 years ago

I've discovered a curious situation. Try the following:

  1. Paste this in the source editor: a[caption align="alignright"]<img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption[/caption]
  2. Switch to visual mode
  3. Delete a so that the image is on the first line
  4. Type a right in front of the image => WORKS
  5. Switch to HTML
  6. Delete the a
  7. Switch to visual mode
  8. Type a right in front of the image => CRASH

Weird eh? It seems the caption at the very beginning puts AztecText into a bad state.

mzorz commented 6 years ago

Weird eh? It seems the caption at the very beginning puts AztecText into a bad state.

It is! I tend to think something is just not rightly handled with spans in the platform. Anyway, just tested on the PR branch and couldn't repro :+1:

mzorz commented 6 years ago

There's even more weird stuff

  1. Paste that same thing in the source editor a[caption align="alignright"]<img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption[/caption]

  2. Switch to visual

  3. Put the cursor right after the "a" and hit backspace

  4. Switch to HTML mode, see this is the content now: <br> [caption align="alignright"]<img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption[/caption] (Look at the <br> up there)

  5. Switch to visual mode

  6. Place the cursor right to the left of the image and hit backspace, see the whole content moves up one line

  7. Switch to HTML

  8. Observe not only the br is gone, but also the [caption] tag is gone as well, leaving an unformatted "Caption" text:

<br> [caption align="alignright"]<img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption[/caption]

Also the undo/redo does weird stuff

mzorz commented 6 years ago

Sorry this is the content observed in step 8 above: <img src="https://examplebloge.files.wordpress.com/2017/02/3def4804-d9b5-11e6-88e6-d7d8864392e0.png">Caption

0nko commented 6 years ago

Tracked by Google here: https://issuetracker.google.com/issues/67102093

mzorz commented 6 years ago

Great - I think we may even get away without "fixing" (hacking around) it.

mzorz commented 6 years ago

Decision: we have to catch the global exception and try to recover from the crash, if it's possible at all.

0nko commented 6 years ago

It has been fixed by Google. It will be released in the next Android version ๐Ÿ˜€.

mzorz commented 6 years ago

It has been fixed by Google. It will be released in the next Android version ๐Ÿ˜€

niiiiiiiceeee ๐ŸŽ‰ ๐Ÿ’ƒ

mzorz commented 5 years ago

Fixed for now I Android 8.x via #801, closing