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

JIRA-38: Jira Macro "list" produces element that includes other prece… #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trrenty
Copy link

@trrenty trrenty commented Jan 27, 2025

…eding elements

Solves issue https://jira.xwiki.org/browse/JIRA-38 and is also beneficial to https://jira.xwiki.org/browse/JIRA-65 which was heavily affected by it. When the user pasted multiple links one after another, the macros would combine in the WYSIWYG view and would disappear on save. Both the enum view and the list view were affected by this.

@vmassol
Copy link
Member

vmassol commented Jan 27, 2025

Thx for the PR. I don't understand why we need to wrap the BulletedListBlock with a GroupBlock since BulletedListBlock is a standalone element. Could you explain?

@vmassol
Copy link
Member

vmassol commented Jan 27, 2025

Also the following is wrong too:

beginGroup
beginTable
beginTableRow
beginTableHeadCell
...
endTableCell
endTableRow
endTable
endGroup

There's no need for a beginGroup/endGroup around a TableBlock since it's standalone too.

Could it be that the issue is with the CKEditor integration rather than in the jira macro?

@trrenty
Copy link
Author

trrenty commented Jan 29, 2025

@vmassol Indeed, looking into the CKEditor issues I found https://jira.xwiki.org/browse/XWIKI-22349 which suggests that the jira macro generates invalid xhtml. Looking at the xhtml generated by the enum view of the jira macro:

<body id="body" class="skin-flamingo viewbody main wiki-xwiki
space-Main">
    <!--startmacro:jira|-|id="teotest" style="enum"|-|JIRA-10--><!--startmacro:jira|-|id="teotest" style="enum"|-|JIRA-10--><!--startimage:true|-|url|-|https://jira.xwiki.org/images/icons/statuses/closed.png--><img
        src="https://jira.xwiki.org/images/icons/statuses/closed.png" data-xwiki-lightbox="false" alt="Closed"
        title="Closed" /><!--stopimage--> <!--startwikilink:true|-|url|-|https://jira.xwiki.org/browse/JIRA-10--><span
        class="wikiexternallink"><a class="wikimodel-freestanding"
            href="https://jira.xwiki.org/browse/JIRA-10"><del><span
                    class="wikimodel-verbatim">JIRA-10</span></del></a></span><!--stopwikilink--><!--stopmacro--><!--stopmacro-->
    <div class="wikimodel-emptyline"></div>
</body>

It seems that it generates a span inside the body element which is invalid (the body does not accept inline elements).
Checking the screenshot present in the issue raised on the jira repo https://jira.xwiki.org/browse/JIRA-38 , i noticed that the user inserted an enum jira macro before the list jira macro. Thus, the title of the ticket is incorrect. I will update it.

However, I still encountered some issues when inserting the other views of the macro, namely in this scenario (note the empty space before the macro):

 {{jira id="teotest" style="list"}}
JIRA-10
{{/jira}}

Using the macro like this results in a render like this
image

Which translates in the following source view

 {{jira id="teotest" style="list"}}JIRA-10{{/jira}}

* [[image:url:https://jira.xwiki.org/images/icons/statuses/closed.png||alt="Closed" data-xwiki-lightbox="false" title="Closed"]] [[--{{{JIRA-10}}}-->>url:https://jira.xwiki.org/browse/JIRA-10]] {{{Add support for displaying JIRA custom fields}}}

The content rendered by the macro seems to have been generated outside of the macro. I will raise an issue describing this. I believe this issue is caused by the fact that the jira macro advertises itself that it supports inline but the list view and table view are not supposed to be used inline.

Tracing back to our issue, JIRA-38, we should only wrap the enum displayer in a group when the macro context is not inline. In order to do that, we need to pass the MacroTransformationContext to the displayer. Does that sound right to you?

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.

2 participants