XRENDERING-771: Null pointer exception when list items occur without lists #327
Conversation
…lists * Protect against list items without wrapping list. * Add tests.
| @@ -278,6 +278,10 @@ public void beginDocument(MetaData metadata) | |||
| public void beginDefinitionDescription() | |||
| { | |||
| ++this.inlineDepth; | |||
| if (this.definitionListDepth.isEmpty()) { | |||
There was a problem hiding this comment.
Why not encapsulating the content of the method in an if (!this.definitionListDepth.isEmpty()) { instead of adding artificially an element? Would that break something else?
There was a problem hiding this comment.
Hm, I thought so, but now I'm not so sure anymore. It seems that getListItemIndex already checks if the stack is non-empty, so this might actually be a better solution... It will break the printing of the list items in both XWiki syntax and plain text, but it might be cleaner.
There was a problem hiding this comment.
And +1 for having a warning in the else branch then.
There was a problem hiding this comment.
I've adjusted the code. I've decided not to log a warning as it is hard to provide relevant information for admins to perform any actions, in particular without a stack trace. For now, I'm just logging a debug message. A possibility could also be to make this a warning when deprecation logging is enabled as in https://github.com/xwiki/xwiki-platform/blob/05d353688490b416a90fcf9ba116cca04e8a9b06/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java#L2434-L2437 but to me that doesn't really seem fitting, what do you think?
There was a problem hiding this comment.
to me that doesn't really seem fitting
Yeah, does not feel like a deprecation. In the case of cache document modification, I used it because modifying directly the cached document used to be kind of officially OK.
…lists * Don't open a (definition) list when a list item is there. * Log a debug message to make it possible to debug this problem. * Adjust the tests for the new behavior.
Jira URL
https://jira.xwiki.org/browse/XRENDERING-771
Changes
Description
Clarifications
Screenshots & Video
No UI changes.
Executed Tests
Expected merging strategy