-
Notifications
You must be signed in to change notification settings - Fork 8
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
The content in the "Text" shape isn't showing in XWiki PDF export when there is text separated on different rows by Enter #118 #146
Conversation
…n there is text separated on different rows by Enter xwikisas#118 * treated separately the cases of partially styled elements and provide fallback to multiple such cases
…n there is text separated on different rows by Enter xwikisas#118 * lines too long * error was raised in java while parsing link+text nodes
{ | ||
String valueInsideLinkElement = StringUtils.substringBetween(value, "<a", "</a>"); | ||
return String.format("<a %s </a>", valueInsideLinkElement); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit hackish. It's not clear to me why you need this since you parse the HTML in getLinkFromEmbeddedNode()
anyway. So it shouldn't matter if there is text before or after the link. Couldn't you just replace:
((Element) doc.getFirstChild()).getAttribute(HREF);
with a more bullet-proof way of getting the anchor element (e.g. using XPath and finding the first anchor element).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an element like https://github.com/xwikisas/application-diagram/pull/146/files#diff-a28d191cad4efd2ff1d7c794c89e41ddR733 were there are text nodes outside html elements
the value that will get here is Simple text <br> <a href="http://localhost:8080/xwiki/bin/view/ATest/">Text</a>
and this is not well formed xml, so it will throw an error in getLinkFromEmbeddedNode.
Thus this method has the purpose to remove any outside element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you just need to wrap value
in <div>
and </div>
to make sure you have a well-formed XML no? And if the problem is the <br>
then it would be safer to use the HTMLCleaner
to parse the HTML.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a fix using HTMLCleaner in my last commit. The problem with it is that there could be multiple a
nodes inside the value
that we get in java, but I would prefer to work on a different issue for that, since it doesn't seems that much related to the title of this issue anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sounds good.
} | ||
if (isFirstChild) { | ||
takenCoordinates = []; | ||
} | ||
w = Math.round(w / 2); | ||
h = Math.round((h + s.fontSize) / 2); | ||
[w, h] = getAdjustedInitialCoordinates(w, h, s.fontSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem to be supported in IE unfortunately.. see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Browser_compatibility .
*/ | ||
var convertPartiallyStyledParagraphToSVG = function(childNodes, s, w, h, htmlConverter, str, rectWidth) { | ||
takenCoordinates = []; | ||
[w, h] = getAdjustedInitialCoordinates(w, h, s.fontSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same IE issue.
…n there is text separated on different rows by Enter xwikisas#118 * updated IE not supporter method * cleaner way to clean the html
// TODO: iterate though children nodes since there could be multiple links inside the given value. | ||
Document doc = defaultHTMLCleaner.clean(new StringReader(value)); | ||
NodeList nodes = doc.getElementsByTagName("a"); | ||
return ((Element) nodes.item(0)).getAttribute(HREF); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would check that the node list is not empty, just to be extra safe.
…n there is text separated on different rows by Enter xwikisas#118 * check if link node exists, to be extra safe (example: there is 'href' string inside a node)
Comparison on what the diagram shows:
and what is on the XWiki PDF Export :