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

Nested html tags use styles of all elements in stack #433

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jakobjoachim
Copy link

While working on fixing Issue #432 I noticed a Bug when using nested spans with styles:

This HTML Code should produce a bold and italic text

<span style="font-weight: bold;">
    <span style="font-style: italic;">
        text
    </span>
</span>

But because only the first item in the stack is used when converting to docx the result is only italic:

<w:r>
    <w:rPr>
        <w:i/>
    </w:rPr>
    <w:t xml:space="preserve">text</w:t>
</w:r>

should be:

<w:r>
    <w:rPr>
        <w:b/>
        <w:i/>
    </w:rPr>
    <w:t xml:space="preserve">text</w:t>
</w:r>

Copy link

@MalteJoe MalteJoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the COMBINED ContainerType.
Otherwise just some formatting issues.


/**
* SAX content handler used to parse HTML content and call the right method of {@link IDocumentHandler} according the
* HTML content.
*/
public class HTMLTextStylingContentHandler
extends DefaultHandler
public class HTMLTextStylingContentHandler extends DefaultHandler

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the formatting changes in the whole file

propertiesList.addAll( spansStack );
ContainerProperties result = null;
for(ContainerProperties properties : propertiesList) {
if (result == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incnsistent formatting (whitespace around braces, curly braces in new line)

public static ContainerProperties combine( ContainerProperties p1, ContainerProperties p2 )
{
ContainerProperties result = new ContainerProperties(
p1.getType() == p2.getType() ? p1.getType() : ContainerType.COMBINED );

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this implementation lead to problems in ODT when combining Paragraphs?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked the ODTDocumentHandler.java currently it doesn't support styles from spans at all. So it really makes no difference. There is probably an Issue or there should be an Issue reading the Span Styles as well and when that is implemented the containerType would be read as well.

@angelozerr
Copy link
Member

@MalteJoe what do you think about this PR?

@MalteJoe
Copy link

MalteJoe commented Mar 7, 2023

@angelozerr As the Formatting Guidelines mentioned in https://github.com/opensagres/xdocreport/wiki/CodeFormatAndConvention are outdated and may be replaced with https://github.com/palantir/palantir-java-format at some point, I will close my comments regarding formatting. There's still one open comment, but it's too old for me to remember what exactly it was about, as I haven't been working with xdocreport for a while now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants