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.

Sign up for GitHub

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

Jump to bottom

ImageObject: contentUrl and .media path #966

Open
rgieseke opened this issue Jun 29, 2021 · 3 comments
Open

ImageObject: contentUrl and .media path #966

rgieseke opened this issue Jun 29, 2021 · 3 comments

Comments

@rgieseke
Copy link
Contributor

After upgrading to 0.117.4 the contentUrl paths of ImageObjects don't contain the sibling path anymore.

They are still copied to the sibling folder (

encoda/src/index.ts

Line 350 in 0a0ed52

// The convention amongst codecs is to put media files in
).
Is this change on purpose (one could always map this i guess)?
@rgieseke
Copy link
Contributor Author

I was wrong, the file is not copied anymore.

E.g. a JATS XML file

<?xml version="1.0"?>
<article>
<front>
<article-meta>
<contrib-group />
</article-meta>
</front>
<body>
<fig id="fig1">
<graphic xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="test.jpg"
id="g1" mimetype="image/jpeg" />
</fig>
</body>
<back>
</back>
</article>

Running

node dist convert test/test.xml --from jats test/out/test.json

yields the following structure:
test
├── out
│ └── test.json
├── test.jpg
└── test.xml

With 0.115.4 running

node dist/cli convert test/test.xml --from jats test/out/test.json

gave

test
├── out
│   ├── test.json
│   └── test.json.media
│   └── test.jpg
├── test.jpg
└── test.xml

@alex-ketch
Copy link
Contributor

Thanks once again for the bug report and the investigative notes @rgieseke, very much appreciated 💖

After looking into this myself, it looks like Encoda's convert function isn't being called when invoked via the CLI, instead it only defers conversion to Jesta. Jesta on the other doesn't know how to handle conversion :(

Our unit tests didn't catch the regression because they mostly bypass this codepath, and instead test the individual codec's convert function.

Unfortunately, I think the best bet for you right now is to downgrade to v0.116.1 while we fix the issues (and add tests to make sure this doesn't happen in the future).

Meanwhile let me know if I can help with anything else!

@rgieseke
Copy link
Contributor Author

rgieseke commented Jul 1, 2021

Thanks for the explanation!
Not sure whether there are other places where this might be tested but in the JATS tests contentUrl was removed here: 227e100#diff-536d58ef076a53c2201d64029419727b9318c2408b7d78dbbf4168bcbb98b3f1

Not sure how exactly the tmp paths there were handled, maybe it's not a problem anymore and are relative now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rgieseke @alex-ketch