fix: Markdown support in listings templates #11760

mcanouil · 2024-12-30T22:35:45Z

The changes restore Markdown support in the default listings templates after the introduction of {=html} in version 1.7.5. This update ensures that Markdown formatting is correctly rendered in listings.

Fixes #11758.

This PR also adds a check for subtitle field in the default listing. Previously it was adding subtitle even when not provided as long as title was.

Tested on: https://m.canouil.dev/quarto-issues-experiments/issue11758/
Source: https://github.com/mcanouil/quarto-issues-experiments/tree/main/quarto-cli-11758

To do:

check that the fix for Website render consumes all memory #11701 still works.

Fixes quarto-dev#11758

src/resources/projects/website/listing/item-default.ejs.md

src/resources/projects/website/listing/item-grid.ejs.md

cderv

Thanks a lot !

Using raw inlines for each raw HTML seems the right call if we need to avoid markdown in HTML parsing.

Only impact I can think of is that <div> in markdown leads to Div in AST, while using raw inline leads to RawInline. It has impact on Lua filter processing.

❯ pandoc -t native
<div class="thumbnail">**STRONG**</div>
^Z
[ Div
    ( "" , [ "thumbnail" ] , [] )
    [ Plain [ Strong [ Str "STRONG" ] ] ]
]

~ took 12s
❯ pandoc -t native
`<div class="thumbnail">`{=html}**STRONG**`</div>`{=html}
^Z
[ Para
    [ RawInline (Format "html") "<div class=\"thumbnail\">"
    , Strong [ Str "STRONG" ]
    , RawInline (Format "html") "</div>"
    ]
]

So we probably need to make sure we have no Lua processing applying on any Div we create in Listing. I check our Lua code using search.

See more in comment about some example of potential issue - I think we are safe as they seems to apply in Dashboard format only.

Regarding test, it seems something needs to be adapted as a test seems to be broken. Didn't look carefully yet.

Thanks again for the PR !

src/resources/projects/website/listing/_pagination.ejs.md

src/resources/projects/website/listing/item-default.ejs.md

src/resources/projects/website/listing/item-grid.ejs.md

mcanouil · 2025-01-02T12:51:13Z

I'm a bit stuck for the remaining raw HTML code.

Using the following without raw code block works as expected for having the item being read/evaluated as markdown.

<h5 class="no-anchor card-title listing-title"><%= item.title %></h5>

Using raw block will lead to the item being placed in a <p> tag leading to extra styling which is really unwanted inside h5.

```{=html}
<h5 class="no-anchor card-title listing-title">
```
<%= item.title %>
```{=html}
</h5>
```
```

This happens for other parts with `<div>` (grid and default listings) and `<td>` (table listing and grid listing for metadata).

Margins added by `<p>` could be dealt with (S)CSS but it's a bit messy.

Any ideas?

mcanouil · 2025-01-02T12:52:31Z

src/resources/projects/website/listing/listing-table.ejs.md

+```
 <%= outputValue(itemNumber, field) %>
+```{=html}


This leads to extra <p> being added as the line is read as Markdown.

refactor a bit by making <td> outside the raw block.

9c43d63

src/resources/projects/website/listing/item-grid.ejs.md

src/resources/projects/website/listing/item-default.ejs.md

mcanouil · 2025-01-02T12:55:30Z

src/resources/projects/website/listing/item-grid.ejs.md

+```{=html}
 <table class="card-other-values">
-<% for (const field of otherFields) { 
-let value = readField(item, field);  
+<% for (const field of otherFields) {
+let value = readField(item, field);
 %>
 <tr>
 <td><%= listing.utilities.fieldName(field) %></td>
-<td class="<%-field%>"><%= listing.utilities.outputLink(item, field, value) %></td>
+<td class="<%- field %>"><%= listing.utilities.outputLink(item, field, value) %></td>
 </tr>
 <% } %>
 </table>
+```


No markdown supports in metadata with this.
Splitting this to allow markdown for listing.utilities.fieldName(field) and listing.utilities.outputLink(item, field, value) will lead to <p> being added.

For consistency until there is a decision, I switched to using raw blocks except for the lines with the variables to allow markdown.

9c43d63

This means, there is still inline HTML.

mcanouil · 2025-01-02T13:03:09Z

In the current state, I have a constant normal memory consumption when rendering the project from #11701

Still need to check the failing test: https://github.com/quarto-dev/quarto-cli/actions/runs/12582608380/job/35068595360?pr=11760
Edit: I don't understand what's the issue or what's being tested as the listing (tests/docs/listings) renders and works properly from what I have seen.
This listing test could be updated to include markdown in title/subtitle/description.
Edit2: seems to be good now

cscheid

This looks great!

I wanted to leave one comment for future reference because of a subtle change in behavior, but I think it's ready to merge.

src/resources/projects/website/listing/item-grid.ejs.md

tests/docs/listings/_quarto.yml

cderv

This is to improve my knowledge and remove some concerns due to interrogation I have. I prefer to be sure we avoid possible regression.

Those EJS template where using <%- and not <%= in some case to be sure that content is encoded, when used in attributes for example.

Maybe there was issue to fix and we wrongly encoded some of them, though I am not sure all replacement of <%- are justified below, specifically those with localizedString().

I may be missing something though, so asking for precision again to fully understand those EJS template.

Thanks for your patience :)

cderv · 2025-01-07T14:07:45Z

src/resources/projects/website/listing/_filter.ejs.md

localizedString() usage here will insert a user provided value. I think it is safer to keep using <%- here, especially when used in HTML attribute, as we can't know which character would be used.

Maybe there won't be any < used for listing-page-order-by but if so, then I think it could break HTML.

That is my thinking. If there is a specific reason, I am missing it, so I would really like to understand and learn something new. Why replacing ?

My first though is that using %<- for escaping is safer.

Thank you

The value is used inside text attribute or inside div which means the content is directly displayed to the users.
Escaping what's supposed to be a string would lead to weird placeholder, aria-label, and option text display.

Why would the text need escaping? I might have missed something but to me, there are no reasons to escape those values.

Both "work" and both can lead to unexpected outputs when non ASCII string provided.
I don't think there is a perfect solution. The rational being the use of = was the same as the overall PR, i.e., allow Markdown/HTML (of course no markdown evaluation is possible anyway when inside raw block).

My understanding of this <%- vs <%= in template is that

Templating here is used to create HTML content

Some characters in HTML have meaning for the structure and so if they need to be shown as text, they should be HTML escape so that in browser they are shown as text, and not considered HTML

Maybe this won't ever happen but I was thinking it is safer to always HTML escape <%- when we know the content provided shouldn't be interpreted as HTML structure.

Example in lodash doc #11787 is quite explicite about escaping

// Use the HTML "escape" delimiter to escape data property values. var compiled = _.template('<b><%- value %></b>'); compiled({ 'value': '<script>' }); // => '<b><script></b>'

If you want to write <script> as such in strong case, then you need the template to HTML escape the value.

So for language customizable value, I am thinking it is better to protect if a user wants to use something like quotes (") in the value.

Maybe I am wrong about this but I think my thinking sum up

<%- would be safer as this is why it was used before. Maybe it is not useful but it does not harm to use it

I don't see the gain of switching to <%=, especially if it could potentially be opened to regression.

🤷

As all the occurrences are in an HTML raw block, I think we can go (back) with <%- as no markdown will ever work there.

Should I do the PR?

To keep on the purpose of learning, as this is still unclear, can you explain to me

no markdown will ever work there.

Or rather you'll confirm if I understand correctly

This ejs template is in fact a markdown file

So we should be carefull when not in a raw block because Markdown in HTML will be interpreted (which is the initial problem)

And in this case, escaping could be harmful because it could break the markdown syntax.

And so what I shared above is really valid only inside raw block, or inside a template file that is not interpreted as .md file.

I think this is what I was missing, right ?

Yes, that's it.

We should probably update the documentation about all this story:

using raw block (Pandoc memory issue and possible parsing issue that could happen, as without, Pandoc has to guess it's HTML)

EJS templates in Quarto are read as Mardown files

https://quarto.org/docs/websites/website-listings-custom.html

I went ahead and made a PR:

fix: restore HTML escape (<%-) for listing.utilities.localizedString() #11817

Also opened a documentation issue:

Improve custom listings documentation to avoid caveats #11818

src/resources/projects/website/listing/_pagination.ejs.md

fix: Markdown is no longer supported in listings (>= 1.7.5)

f9a5250

Fixes quarto-dev#11758

mcanouil self-assigned this Dec 30, 2024

revert: unwanted change

4b4c36b

mcanouil commented Dec 30, 2024

View reviewed changes

src/resources/projects/website/listing/item-default.ejs.md Show resolved Hide resolved

mcanouil commented Dec 30, 2024

View reviewed changes

src/resources/projects/website/listing/item-default.ejs.md Outdated Show resolved Hide resolved

mcanouil commented Dec 30, 2024

View reviewed changes

src/resources/projects/website/listing/item-grid.ejs.md Show resolved Hide resolved

mcanouil commented Dec 30, 2024

View reviewed changes

src/resources/projects/website/listing/item-grid.ejs.md Outdated Show resolved Hide resolved

mcanouil changed the title ~~fix:Markdown support in listings templates~~ fix: Markdown support in listings templates Dec 30, 2024

cderv reviewed Dec 31, 2024

View reviewed changes

cderv mentioned this pull request Dec 31, 2024

Listing page categories are not working properly if spaces or non ascii chracters #11752

Closed

mcanouil added 9 commits January 2, 2025 11:43

fix: use markdown div thus no raw block

0f779cf

refactor: use markdown syntax

911207e

revert: use raw block to avoid "p"

8463bc7

fix: typo in div code

0af2dd7

refactor: put if/else inside code block

5d1ef85

fix: use raw block for localizedString and don't escape content

0aafe42

fix: use raw block instead of inline

43ff451

fix: no need to escape

f03ee1e

fix: no need to escape base64 value

d52e2ba

mcanouil commented Jan 2, 2025

View reviewed changes

mcanouil added 2 commits January 2, 2025 14:11

test: styles.sccs does not exist

2b73ea2

Merge branch 'main' into fix/issue11758

61094dd

mcanouil added the needs-discussion Issues that require a team-wide discussion before proceeding further label Jan 2, 2025

fix: ensure no extra<p> tag is added

9c43d63

mcanouil marked this pull request as ready for review January 2, 2025 16:26

cscheid approved these changes Jan 6, 2025

View reviewed changes

src/resources/projects/website/listing/item-grid.ejs.md Show resolved Hide resolved

tests/docs/listings/_quarto.yml Show resolved Hide resolved

cscheid merged commit 7417a5e into quarto-dev:main Jan 6, 2025
47 checks passed

mcanouil deleted the fix/issue11758 branch January 6, 2025 16:26

cderv reviewed Jan 7, 2025

View reviewed changes

This was referenced Jan 8, 2025

fix: restore HTML escape (<%-) for listing.utilities.localizedString() #11817

Open

Improve custom listings documentation to avoid caveats #11818

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Markdown support in listings templates #11760

fix: Markdown support in listings templates #11760

mcanouil commented Dec 30, 2024 •

edited

Loading

cderv left a comment

mcanouil commented Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil commented Jan 2, 2025 •

edited

Loading

cscheid left a comment

cderv left a comment

cderv Jan 7, 2025

mcanouil Jan 7, 2025 •

edited

Loading

cderv Jan 7, 2025

mcanouil Jan 7, 2025

cderv Jan 7, 2025

mcanouil Jan 7, 2025

mcanouil Jan 7, 2025 •

edited

Loading

mcanouil Jan 8, 2025

mcanouil Jan 8, 2025

fix: Markdown support in listings templates #11760

fix: Markdown support in listings templates #11760

Conversation

mcanouil commented Dec 30, 2024 • edited Loading

cderv left a comment

Choose a reason for hiding this comment

mcanouil commented Jan 2, 2025 • edited Loading

mcanouil Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

mcanouil Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcanouil Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

mcanouil commented Jan 2, 2025 • edited Loading

cscheid left a comment

Choose a reason for hiding this comment

cderv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcanouil Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcanouil Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcanouil commented Dec 30, 2024 •

edited

Loading

mcanouil commented Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil Jan 2, 2025 •

edited

Loading

mcanouil commented Jan 2, 2025 •

edited

Loading

mcanouil Jan 7, 2025 •

edited

Loading

mcanouil Jan 7, 2025 •

edited

Loading