search

eclipse / nebula

Nebula Project
https://eclipse.org/nebula
Eclipse Public License 2.0
84 stars 98 forks source link

RichTextPainter fails to render valid XHTML in case the QName has a namespace URI #500

Closed jmauersberger closed 1 year ago

jmauersberger commented 1 year ago

I stumbled over this when rendering XHTML from other sources. Lets say the XHTML starts with

<div xmlns="http://www.w3.org/1999/xhtml">

In that case code at https://github.com/eclipse/nebula/blob/2f10d093e081a0bdfef36716700b929f43e1876a/widgets/richtext/org.eclipse.nebula.widgets.richtext/src/org/eclipse/nebula/widgets/richtext/RichTextPainter.java#L231 will evaluate the QName.toString() value wich is {http://www.w3.org/1999/xhtml}div and obviously not div so any code after that wont work correctly. I guess a fix is pretty straight, just use QName.getLocalPart(), right? Or was that done by intention to ONLY parse XHTML without namespace?

fipro78 commented 1 year ago

Well, the RichTextPainter was created as a renderer for content that was created by the RichTextEditor. I never came across a scenario where a namespace was included.

Was the code created this way by intention? Honestly I can't remember. We would need some extensive testing to ensure that the proposed fix does not break anything else.

jmauersberger commented 1 year ago

Thanks Dirk for the fast response. Well, we get these XHTML snippets from other tools like DNG, Codebeamer, Jama etc. they usually deliver bad or at least special HTML (although recently it looks as if some are using the same editing control that Nebula embeds). In the past it wasn't even sometimes real HTML. Anyway, looking at toString() which just concatenates the local part with the namespace, I bet its pretty much safe to rely on the local part only, at least that should be "backward compatible" and work as before for any kind of XHTML created by Nebula. I can offer help to work in this project but I first need to do some paperwork here on my end to be able to contribute to an OS project as this one.

wimjongman commented 1 year ago

Welcome, Jan.

fipro78 commented 1 year ago

Fixed with https://github.com/eclipse/nebula/pull/506

I did some testing and checked the implementation details. I did not see any side effects and the results look as expected.

Thanks for reporting and proposing a fix. Even it was only in textual form and not in form as a PR. :)

jmauersberger commented 1 year ago

Awesome, thanks Dirk. Actually I have a PR ready in our clone but legal did not yet gave me final confirmation so we could have contributed. You have been faster :-)