wordpress-mobile / AztecEditor-Android

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

Fix for 'PARAGRAPH span must start at paragraph boundary' #539

Closed daniloercoli closed 6 years ago

daniloercoli commented 6 years ago

Fix #501 and #481 by not adding new lines to empty spans.

To Test Paste the following HTML code in the visual editor

<p></p><span>[caption]</span>

It should not crash the app.

There is a change that the same problem can occur for other tags that are not "caption" or "img", but I wan't able to reproduce on those other tags.

0nko commented 6 years ago

It seems this does not fix the issue entirely. Pasting the full text from https://fabric.io/automattic/android/apps/org.wordpress.android/issues/59a7e418be077a4dcce162ef?time=last-thirty-days still causes a crash.

Maybe we should add a condition check before applying SPAN_PARAGRAPH flag in AztecParser.

daniloercoli commented 6 years ago

Right, it's probably best to add a condition check before applying SPAN_PARAGRAPH.

For reference this is a test code to use to test this other problem: <p></p> <span> [caption ]</span>Test

daniloercoli commented 6 years ago

@0nko I've added the checks to SpanWrapper to ensure the PARAGRAPH span is valid (copied from Android). We may want to log the error though. Let me know what are your thoughts on this, since I don't see any other part of the Editor using Log in production.

0nko commented 6 years ago

Would the system Log messages show up anywhere if the app doesn't crash? I know you can log caught exceptions in Crashlytics but we'd be avoiding that here.

daniloercoli commented 6 years ago

Nope, system Logs are not shown in wp-android app logs. We should use our Logging library AppLog, already available in Utils, if we want to add them to the main app log.

Btw, this is ready for another round.

0nko commented 6 years ago

Thanks. It looks good 👍. I'll merge this one and we can add logging in another PR, I think it could be useful.

:shipit: