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

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

Closed
0nko opened this issue Nov 6, 2017 · 17 comments
Closed

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

0nko opened this issue Nov 6, 2017 · 17 comments

Comments

@0nko
Copy link
Contributor

0nko commented Nov 6, 2017

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
Copy link
Contributor

mzorz commented Nov 15, 2017

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 mzorz self-assigned this Nov 15, 2017
@mzorz
Copy link
Contributor

mzorz commented Nov 15, 2017

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 mzorz changed the title Crash when applying special comments on API 27 Crash when applying special comments on API 26/27 Nov 16, 2017
@mzorz
Copy link
Contributor

mzorz commented Nov 16, 2017

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

@mzorz
Copy link
Contributor

mzorz commented Nov 22, 2017

Managed to narrow this down to:

  • if the Caption tag is not empty, every time you have an image tag with a Caption and attempt to insert a Special comment (or for the matter, the "native" Horizontal Rule works as well) right before it, it crashes.

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
Copy link
Contributor

mzorz commented Nov 22, 2017

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 668dcfb :

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?

@maxme maxme added this to the 1.0 milestone Nov 23, 2017
@0nko
Copy link
Contributor Author

0nko commented Nov 23, 2017

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
Copy link
Contributor

mzorz commented Nov 23, 2017

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
Copy link
Contributor Author

0nko commented Nov 24, 2017

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
Copy link
Contributor

mzorz commented Nov 24, 2017

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 👍

@mzorz
Copy link
Contributor

mzorz commented Nov 24, 2017

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
Copy link
Contributor

mzorz commented Nov 24, 2017

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
Copy link
Contributor Author

0nko commented Nov 28, 2017

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

@mzorz
Copy link
Contributor

mzorz commented Nov 28, 2017

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

@mzorz
Copy link
Contributor

mzorz commented Dec 6, 2017

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

@0nko
Copy link
Contributor Author

0nko commented Dec 20, 2017

It has been fixed by Google. It will be released in the next Android version 😀.

@mzorz
Copy link
Contributor

mzorz commented Dec 20, 2017

It has been fixed by Google. It will be released in the next Android version 😀

niiiiiiiceeee 🎉 💃

@mzorz
Copy link
Contributor

mzorz commented Apr 16, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants