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

Comment notification with certain HTML crashes HTML.fromHTML #334

Closed
beaucollins opened this issue Nov 21, 2013 · 4 comments · Fixed by #541 or #598
Closed

Comment notification with certain HTML crashes HTML.fromHTML #334

beaucollins opened this issue Nov 21, 2013 · 4 comments · Fixed by #541 or #598
Assignees
Milestone

Comments

@beaucollins
Copy link
Contributor

A comment with a <p> inside an <li> will throw a RuntimeException.

<li><p>Some content</p></li>
@beaucollins
Copy link
Contributor Author

I/TestRunner( 4190): java.lang.RuntimeException: PARAGRAPH span must start at paragraph boundary
I/TestRunner( 4190):    at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:600)
I/TestRunner( 4190):    at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:588)
I/TestRunner( 4190):    at android.text.HtmlToSpannedConverter.convert(Html.java:462)
I/TestRunner( 4190):    at android.text.Html.fromHtml(Html.java:139)
I/TestRunner( 4190):    at org.wordpress.android.models.Note.prepareHtml(Note.java:257)
I/TestRunner( 4190):    at org.wordpress.android.models.Note.cleanupComment(Note.java:181)
I/TestRunner( 4190):    at org.wordpress.android.models.Note.<init>(Note.java:48)
I/TestRunner( 4190):    at org.wordpress.android.ui.notifications.NotesParseTest.testParseNotes(NotesParseTest.java:36)
I/TestRunner( 4190):    at java.lang.reflect.Method.invokeNative(Native Method)
I/TestRunner( 4190):    at java.lang.reflect.Method.invoke(Method.java:525)
I/TestRunner( 4190):    at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
I/TestRunner( 4190):    at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
I/TestRunner( 4190):    at junit.framework.TestCase.runBare(TestCase.java:134)
I/TestRunner( 4190):    at junit.framework.TestResult$1.protect(TestResult.java:115)
I/TestRunner( 4190):    at junit.framework.TestResult.runProtected(TestResult.java:133)
I/TestRunner( 4190):    at junit.framework.TestResult.run(TestResult.java:118)
I/TestRunner( 4190):    at junit.framework.TestCase.run(TestCase.java:124)
I/TestRunner( 4190):    at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
I/TestRunner( 4190):    at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
I/TestRunner( 4190):    at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:554)
I/TestRunner( 4190):    at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1701)

@maxme
Copy link
Contributor

maxme commented Nov 21, 2013

Other tests are in the tests/java/ folder, we should keep everything in the same place (maybe move all tests to tests/src/ makes more sense).

@beaucollins
Copy link
Contributor Author

That was my mistake. New test has bee put in the correct location.

@ghost ghost assigned maxme Dec 18, 2013
roundhill added a commit that referenced this issue Dec 20, 2013
…crash

fix #334: modify our WPHtmlTagHandler to support paragraph tag in a list item
@roundhill
Copy link
Contributor

This is still happening in beta 2. Here's another trace:

java.lang.RuntimeException: PARAGRAPH span must start at paragraph boundary
at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:600)
at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:588)
at android.text.HtmlToSpannedConverter.convert(Html.java:471)
at android.text.Html.fromHtml(Html.java:139)
at org.wordpress.android.models.Note.prepareHtml(Note.java:404)
at org.wordpress.android.models.Note.preloadContent(Note.java:314)
at org.wordpress.android.models.Note.<init>(Note.java:76)
at org.wordpress.android.ui.notifications.NotificationUtils.parseNotes(NotificationUtils.java:64)
at org.wordpress.android.ui.notifications.NotificationsActivity$NotesResponseHandler.onResponse(NotificationsActivity.java:551)
at org.wordpress.android.ui.notifications.NotificationsActivity$NotesResponseHandler.onResponse(NotificationsActivity.java:532)
at org.wordpress.android.ui.notifications.NotificationUtils$1.onResponse(NotificationUtils.java:44)
at org.wordpress.android.ui.notifications.NotificationUtils$1.onResponse(NotificationUtils.java:40)
at com.wordpress.rest.RestRequest.deliverResponse(RestRequest.java:60)
at com.wordpress.rest.RestRequest.deliverResponse(RestRequest.java:17)
at com.android.volley.ExecutorDelivery$ResponseDeliveryRunnable.run(ExecutorDelivery.java:99)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:136)
at android.app.ActivityThread.main(ActivityThread.java:5017)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:515)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595)
at dalvik.system.NativeStart.main(Native Method)

@roundhill roundhill reopened this Dec 23, 2013
maxme added a commit that referenced this issue Dec 26, 2013
roundhill added a commit that referenced this issue Jan 6, 2014
…-crash-with-certain-html

Fix #334 notifications crash with certain html

Reviewed last commit with @aerych.
maxme added a commit that referenced this issue Jan 13, 2014
maxme added a commit that referenced this issue Apr 6, 2016
0f6fba8 Merge pull request #334 from wordpress-mobile/issue/292-backspace-bug
041f285 Changed ZSSEditor.getYCaretInfo to find parent contenteditable divs correctly
92cb325 Fixed an issue with losing cursor focus on API<19
0c2b5fd Made blockquote parsing handle DIVs tags as paragraphs
c62ce8e Convert divs to paragraph tags when extracting the HTML from the editor
f5e5d26 Changed getFocusedField to use this.focusedField, in order to avoid div targeting issues now that paragraphs are also divs
c925ebd Add line height and margin styling to div tags (duplicated from p tags, and excluding the contenteditable and separator divs
eb1677c Drop defaultParagraphSeparator assignment and use divs instead of ps for paragraphs (WebView default)
5e90578 Merge pull request #333 from wordpress-mobile/issue/258-remove-failed-uploads-when-switching-to-html-mode
9a0ef46 Merge commit '4c9324cf1eee00b66c76e0d5a917c86e1293a845' into develop
ac96e74 fix #258: prompt the user to delete failed uploads before switching to HTML mode
b856f7b update to android-gradle 2.0.0-rc1
e6f4e6b Add missing classes for images inserted from media library - also fix a bug with undefined alt text
31d2970 Add missing attributes for uploaded images
ce6ab9d update to android-gradle-2.0.0-rc1

git-subtree-dir: libs/editor
git-subtree-split: 0f6fba8fc434c4007c7cdd300c4dd9fa2df5a218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment