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

Importing HTML with Lexical editor selected results in loss of images. #18448

Closed
1 task done
cathysarisky opened this issue Oct 3, 2023 · 10 comments
Closed
1 task done
Assignees
Labels
affects:editor Work relating to the Koenig Editor bug [triage] something behaving unexpectedly

Comments

@cathysarisky
Copy link
Contributor

cathysarisky commented Oct 3, 2023

Issue Summary

Importing HTML that includes image links, using the Admin API. (Images previously imported with the API images endpoint.)
If I select the Lexical editor (in the admin interface), images get stripped out. (They're not broken links - the img tag is totally missing.)
If I turn off the Lexical editor, images appear correctly.
No error messages are thrown on import, and the rest of the post imports intact.

Steps to Reproduce

This is a regression for the new Lexical editor compared to the old Mobiledoc editor.
Please see comment below for full repro, demonstrated on Ghost Pro 5.68+moya.

Ghost Version

Ghost Pro, as of 10/2/23

Node.js Version

Ghost Pro, as of 10/2/23

How did you install Ghost?

Ghost Pro, as of 10/2/23

Database type

MySQL 8

Browser & OS version

not relevant

Relevant log / error output

no errors throw

Code of Conduct

  • I agree to be friendly and polite to people in this repository
@github-actions github-actions bot added the needs:triage [triage] this needs to be triaged by the Ghost team label Oct 3, 2023
@kevinansfield
Copy link
Contributor

kevinansfield commented Oct 3, 2023

Importing HTML that includes image links, using the Admin API. (Images previously imported with the API images endpoint.)
If I select the Lexical editor (in the admin interface), images get stripped out. (They're not broken links - the img tag is totally missing.)
If I turn off the Lexical editor, images appear correctly.

Hi @cathysarisky, to clarify this point do you mean importing with the beta flag on vs off or do you mean after importing the beta flag has some effect when toggled?

Could you provide a short example of the HTML you are importing please?

@cathysarisky
Copy link
Contributor Author

The beta flag seems to have an effect on import. If I turn it on, the import fails.
Example HTML to follow.

@9larsons
Copy link
Contributor

9larsons commented Oct 6, 2023

I was able to import fine applying a link to the image as well as a link in the caption.

Sample body for the API request:

{
    "posts": [
        {
            "title": "Image Link Import Testing",
            "status": "published",
            "html": "<figure class=\"kg-card kg-image-card kg-card-hascaption\"><a href=\"imageLink\"><img src=\"__GHOST_URL__/content/images/2023/10/2560px-Mandolin_guitar_band_crystal_palace-1.jpg\" class=\"kg-image\" alt=\"\" loading=\"lazy\" width=\"2000\" height=\"1526\" srcset=\"__GHOST_URL__/content/images/size/w600/2023/10/2560px-Mandolin_guitar_band_crystal_palace-1.jpg 600w, __GHOST_URL__/content/images/size/w1000/2023/10/2560px-Mandolin_guitar_band_crystal_palace-1.jpg 1000w, __GHOST_URL__/content/images/size/w1600/2023/10/2560px-Mandolin_guitar_band_crystal_palace-1.jpg 1600w, __GHOST_URL__/content/images/size/w2400/2023/10/2560px-Mandolin_guitar_band_crystal_palace-1.jpg 2400w\" sizes=\"(min-width: 720px) 720px\"></a><figcaption><span style=\"white-space: pre-wrap;\">caption </span><a href=\"captionLink\" rel=\"noreferrer\"><span style=\"white-space: pre-wrap;\">link</span></a></figcaption></figure>"
        }
    ]
}

@cathysarisky could you provide the html string you were using?

@kevinansfield
Copy link
Contributor

@9larsons was this with the latest release containing parser improvements?

@cathysarisky if you can, please try updating to the most recent release and try again to see if the issue has been fixed

@cathysarisky
Copy link
Contributor Author

cathysarisky commented Oct 6, 2023 via email

@9larsons
Copy link
Contributor

9larsons commented Oct 6, 2023

Yes, this was using the most recent release - it's possible this was fixed with the changes I just pushed with that. I can't say for certain without knowing the html string. I don't feel it's likely it was resolved by the changes I made. Please just give the new release a shot and we'll find out!

@9larsons
Copy link
Contributor

@cathysarisky hey - did you get a chance to try this out again?

@cathysarisky
Copy link
Contributor Author

OK, trying on Ghost Pro, so that's version: 5.68.0+moya

The beta editor slider is gone, and the problem occurs.

Here's a fairly minimalist repro case:

let ghostAPI=process.env.ghostAPI;
let ghostKey=process.env.ghostKey;

const api = new GhostAdminAPI({
    url: ghostAPI,
    version: "v5.0",
    key: ghostKey
});

let html = `
<html><head></head><body><p>Here is a post</p>
<p>Of course, it's a little complicated -- otherwise it wouldn't be so amusing.</p>

<p><img alt="Great Pyramid of Giza" class="asset  asset-image" src="https://demo.spectralwebservices.com/content/images/2023/10/Events-Page.png\" title="Random image from my work"></p>
</body></html>
`

api.posts
.add(
    {title: 'Hello World',
        tags: ['only testing'], 
        status: 'published', 
        html: html,
        slug: 'slug-it-out'
    }, {source: 'html'}
).then((response) => {console.log('response is', response)}).catch((error) => {console.log('error is', error)})

This API call happens correctly, but there's a problem somewhere.

Here's the lexical returned as part of the post object created above:

      lexical: `{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"Here is a post","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1},{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"Of course, it's a little complicated -- otherwise it wouldn't be so amusing.","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1},{"children":[{"type":"image","version":1,"src":"https://demo.spectralwebservices.com/content/images/2023/10/Events-Page.png","width":null,"height":null,"title":"Random image from my work","alt":"Great Pyramid of Giza","caption":"","cardWidth":"regular","href":""}],"direction":null,"format":"","indent":0,"type":"paragraph","version":1}],"direction":null,"format":"","indent":0,"type":"root","version":1}}`,

Upon viewing, the image is not present. Here's the relevant HTML output upon visiting the post.

<section class="gh-content gh-canvas is-body">
            <p>Here is a post</p><p>Of course, it's a little complicated -- otherwise it wouldn't be so amusing.</p>
</section>

If I load the post in the Ghost post editor and republish it, then I get this:

<section class="gh-content gh-canvas is-body">
            <p>Here is a post</p><p>Of course, it's a little complicated -- otherwise it wouldn't be so amusing.</p><p></p><figure class="kg-card kg-image-card"><img src="https://demo.spectralwebservices.com/content/images/2023/10/Events-Page.png" class="kg-image" alt="Great Pyramid of Giza" loading="lazy" title="Random image from my work"></figure>
        </section>

... which is what I wanted in the first place!

With the removal of the old editor, I don't have a workaround for this bug.

@kevinansfield
Copy link
Contributor

kevinansfield commented Oct 17, 2023

Thanks @cathysarisky. Looks like this issue is related to the parsing generating an invalid tree state with an image card node nested inside a paragraph node.

We've added transforms to rectify this state to the editor itself which is why opening the editor and re-publishing works but we're missing those transforms on the server-side html->lexical route. We were actually talking about this yesterday before seeing your reply and have a plan so it should be resolved in the next Ghost release.

@kevinansfield kevinansfield self-assigned this Oct 17, 2023
@kevinansfield kevinansfield added bug [triage] something behaving unexpectedly affects:editor Work relating to the Koenig Editor and removed needs:triage [triage] this needs to be triaged by the Ghost team labels Oct 17, 2023
@kevinansfield
Copy link
Contributor

Short-term workaround would be not wrapping images in paragraphs in the source HTML if that's possible.

kevinansfield added a commit to kevinansfield/Koenig that referenced this issue Oct 17, 2023
…alid nesting

refs TryGhost/Ghost#18448

- extracted our aligment stripping and denesting transforms to a new `kg-default-transforms` package
- updated `kg-html-to-lexical` so it adds the transforms to the headless editor ensuring the output matches what we'd get if the same content was pasted directly into the editor
kevinansfield added a commit to kevinansfield/Koenig that referenced this issue Oct 18, 2023
…alid nesting

refs TryGhost/Ghost#18448

- extracted our aligment stripping and denesting transforms to a new `kg-default-transforms` package
- updated `kg-html-to-lexical` so it adds the transforms to the headless editor ensuring the output matches what we'd get if the same content was pasted directly into the editor
kevinansfield added a commit to kevinansfield/Koenig that referenced this issue Oct 18, 2023
…alid nesting

refs TryGhost/Ghost#18448

- extracted our aligment stripping and denesting transforms to a new `kg-default-transforms` package
- updated `kg-html-to-lexical` so it adds the transforms to the headless editor ensuring the output matches what we'd get if the same content was pasted directly into the editor
kevinansfield added a commit to kevinansfield/Koenig that referenced this issue Oct 19, 2023
…alid nesting

refs TryGhost/Ghost#18448

- extracted our aligment stripping and denesting transforms to a new `kg-default-transforms` package
- updated `kg-html-to-lexical` so it adds the transforms to the headless editor ensuring the output matches what we'd get if the same content was pasted directly into the editor
kevinansfield added a commit to TryGhost/Koenig that referenced this issue Oct 19, 2023
…alid nesting (#1002)

refs TryGhost/Ghost#18448

- extracted our aligment stripping, de-nesting, and list-merging transforms to a new `kg-default-transforms` package
  - generalised the de-nesting transform further to handle invalid nesting inside lists (allowed removal of image-specific denest transform in `koenig-lexical`)
  - updated `koenig-lexical` to use the new package in place of the extracted transform code
- updated `kg-html-to-lexical` so it adds the transforms to the headless editor ensuring the output matches what we'd get if the same content was pasted directly into the editor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:editor Work relating to the Koenig Editor bug [triage] something behaving unexpectedly
Projects
None yet
Development

No branches or pull requests

3 participants