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

SVG are loosing classes when using embed-resources, impacting revealjs features (class="slide-logo", .absolute) #9803

Closed
mcanouil opened this issue May 30, 2024 · 11 comments · Fixed by #11238
Assignees
Labels
bug Something isn't working pandoc regression Functionality that used to work but now is broken. revealjs Issues with the revealjs format
Milestone

Comments

@mcanouil
Copy link
Collaborator

mcanouil commented May 30, 2024

Using Quarto 122b0f3
It works as expected using v1.4.554

Expected behaviour

Quarto documentHTML
---
title: "Quarto Playground"
format:
  revealjs:
    logo: https://raw.githubusercontent.com/mcanouil/hex-stickers/main/SVG/rlille.svg
    embed-resources: false
---

## Slide

This is a playground for Quarto.

![An image](https://placehold.co/600x400.png)
image

Actual behaviour

Quarto documentHTML
---
title: "Quarto Playground"
format:
  revealjs:
    logo: https://raw.githubusercontent.com/mcanouil/hex-stickers/main/SVG/rlille.svg
    embed-resources: true
---

## Slide

This is a playground for Quarto.

![An image](https://placehold.co/600x400.png)
image

The class="slide-logo" is lost when embedding.

@mcanouil mcanouil added bug Something isn't working revealjs Issues with the revealjs format triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. labels May 30, 2024
@mcanouil mcanouil added the regression Functionality that used to work but now is broken. label May 30, 2024
@cderv
Copy link
Collaborator

cderv commented May 31, 2024

This is Pandoc issue IMO so if it was working before probably following an update of pandoc

Try

❯ quarto pandoc --to html --embed-resources -o embed.html
<img src="https://raw.githubusercontent.com/mcanouil/hex-stickers/main/SVG/rlille.svg" class="slide-logo">
^Z

And look in embed.html - the class is missing.

@cderv
Copy link
Collaborator

cderv commented May 31, 2024

But doing the same with Quarto 1.4.554 works - so this is a change in Pandoc.

@cderv
Copy link
Collaborator

cderv commented May 31, 2024

1.4.554 uses pandoc 3.1.11.1 and latest use pandoc 3.2

And we can determine that change happens in Pandoc 3.2

library(pandoc)

min <- "3.1.11"
max <- "3.2"


versions <- pandoc::pandoc_available_releases()
#> ℹ Fetching Pandoc releases info from github...
versions <- versions[seq.int(which(versions == min), which(versions == max))]

for(version in versions) {
  if (!pandoc::pandoc_is_installed(version)) {
    pandoc::pandoc_install(version)
  }
  embed <- pandoc::pandoc_convert(
    text = '<img src="https://raw.githubusercontent.com/mcanouil/hex-stickers/main/SVG/rlille.svg" class="slide-logo">',
    to = "html",
    args = "--embed-resources", 
    version = version)
  if (!any(grepl("slide-logo", embed))) {
    cat("pandoc version ", version, " does not keep classes")
    break
  }
}
#> pandoc version  3.2  does not keep classes

Created on 2024-05-31 with reprex v2.1.0

@cderv
Copy link
Collaborator

cderv commented May 31, 2024

Probably related to @cscheid PR in pandoc

so we may have broken ourself Quarto through Pandoc by trying to fix another issue 😅

Though I don't see how merging would mess this up, because we should have kept the class.
So either if no class on the svg, it does not keep image class.

Or this is a side effect of this change somewhere else

@cderv
Copy link
Collaborator

cderv commented May 31, 2024

Testing with another svg from pandoc repo

❯ quarto pandoc --to html --embed-resources -o embed.html
```{=html}
<img class="something" src="https://raw.githubusercontent.com/jgm/pandoc/7abfec3d2c0e6084b8029d38cd45912697822f3a/test/command/SVG_logo.svg" />
```
^Z

I don't get the class name either

<svg id="svg_7e8f68c20ccd090c0566" role="img" viewBox="-50 -50 100 100" width="100" height="100">
<title>SVG Logo</title>

So it seems when no class on the svg, the one from img tag is lost.

@cscheid I don't know Haskell enough to know if your change is supposed to handle this case.

@cderv cderv assigned cscheid and unassigned cderv May 31, 2024
@cderv cderv added the pandoc label May 31, 2024
@cscheid cscheid added this to the v1.5 milestone Jun 5, 2024
@cscheid
Copy link
Collaborator

cscheid commented Jun 20, 2024

Hm - if this is a Pandoc regression (even one I introduced 🤦) we won't be able to fix it for 1.5.

@cscheid cscheid modified the milestones: v1.5, v1.6 Jun 20, 2024
@cderv
Copy link
Collaborator

cderv commented Jun 21, 2024

What about doing a "hack" to at least restore behavior ? I have a branch working with the following

  • wrap the logo image in a span of a class we know.
  • Leverage revealjs plugin system to restore the class on the element in the span when it is missing.

This would not solve the underlying svg issue, but this would "hide" this issue until we have a proper fix. I can make a PR with the idea.

@cscheid
Copy link
Collaborator

cscheid commented Jun 21, 2024

I don't know. This would only fix it in logo and format: revealjs, right? The bug would still exist in other usages of embed-resources. I'm not sure it's worth the extra complexity in the code base, that would then need to be reverted once upstream is fixed.

@cderv
Copy link
Collaborator

cderv commented Jun 21, 2024

This would only fix it in logo and format: revealjs, right?

Yes exactly.

I'm not sure it's worth the extra complexity in the code base, that would then need to be reverted once upstream is fixed.

That is the question. I'll share the idea in a PR and you'll be the judge of that.

@cderv
Copy link
Collaborator

cderv commented Sep 2, 2024

Here is one use case where another class is lost: When using .absolute class for positioning on an svg element. See

---
title: test SVG
format: revealjs
embed-resources: true
---

## Slide 1

![](https://quarto.org/docs/presentations/revealjs/demo/mini/images/kitten-450-250.jpeg){.absolute top=50 right=50 width="450" height="250"}

![](https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/410.svg){.absolute top=50 right=50 width="450" height="250"}

@cderv cderv changed the title embed-resources removes class="slide-logo" from the logo in format: revealjs SVG are loosing classes when using embed-resources, impacting revealjs features (class="slide-logo", .absolute) Sep 2, 2024
@ZokszY
Copy link

ZokszY commented Oct 29, 2024

I was having a similar issue with this template on version 1.5.57:

---
title: "Quarto Playground"
format:
  revealjs:
    embed-resources: true
---

## No width

::: {layout-ncol=2}
![Simple Linear Regression](https://upload.wikimedia.org/wikipedia/commons/e/ec/Anscombe%27s_quartet_3.svg)

![K-NN Classification](https://upload.wikimedia.org/wikipedia/commons/e/e7/KnnClassification.svg)
:::

## width="200px"

::: {layout-ncol=2}
![Simple Linear Regression](https://upload.wikimedia.org/wikipedia/commons/e/ec/Anscombe%27s_quartet_3.svg){width="200px"}

![K-NN Classification](https://upload.wikimedia.org/wikipedia/commons/e/e7/KnnClassification.svg){width="200px"}
:::

## width="500px"

::: {layout-ncol=2}
![Simple Linear Regression](https://upload.wikimedia.org/wikipedia/commons/e/ec/Anscombe%27s_quartet_3.svg){width="500px"}

![K-NN Classification](https://upload.wikimedia.org/wikipedia/commons/e/e7/KnnClassification.svg){width="500px"}
:::

In this example there are two SVG files with different behavior. The first one is properly resized and almost properly placed on every slide, while the second is not resized and has a wrong position except in the last slide. I then tried using using v1.4.554 which didn't help. However the latest version v1.6.32 does work properly.

Here are the results I get with each version (exported to PDF using the PDF Export Mode of RevealJS):

cderv added a commit that referenced this issue Oct 29, 2024
@cderv cderv removed the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Oct 29, 2024
@cderv cderv self-assigned this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pandoc regression Functionality that used to work but now is broken. revealjs Issues with the revealjs format
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants