search

pkp / ots

PKP XML Parsing Service
GNU General Public License v3.0
32 stars 19 forks source link

Images path should be relative with the XML file directory. #131

Closed fabiobatalha closed 6 years ago

fabiobatalha commented 6 years ago

The images path in the final XML seems to have an OTS internal structure, it is a implementation detail that should not be in the final XML file.

screenshot from 2018-03-09 10-14-04

axfelix commented 6 years ago

Yup, this is definitely an issue. @kaschioudi , I thought we implemented relative links across the board for images, do you want to investigate?

kaschioudi commented 6 years ago

@axfelix : at some point we modified images links to match Texture expectations. (see https://github.com/pkp/ots/issues/84#issuecomment-249746270) . Should I revert that change?

axfelix commented 6 years ago

With Texture's new approach to using a manifest to indicate file paths, hopefully it's possible to go back to always using image paths relative to the document.xml, so that we can revert that change and still support Texture?

axfelix commented 6 years ago

We also might need to be exposing the output images more clearly right now via our API (I think right now the easiest way for a user to get them would be to request the html.zip and extract them from there, which is OK but not great).

axfelix commented 6 years ago

Though I don't think we'd be reverting that old commit exactly, because if I remember correctly it was partly intended to make image output paths identical between meTypeset and Cermine so there wouldn't be any meaningless differences in paths between Word and PDF input

kaschioudi commented 6 years ago

With Texture's new approach to using a manifest to indicate file paths, hopefully it's possible to go back to always using image paths relative to the document.xml, so that we can revert that change and still support Texture?

In my branch I have reverted the change for now https://github.com/pkp/ots/blob/ea82e8a3713e863cb829ca69c8f979eac7a80e81/module/Manager/src/Manager/Controller/ManagerController.php#L309-L315)

We also might need to be exposing the output images more clearly right now via our API

We are already doing that. Images are available from /manager/xml/id/:id/media/:file

Though I don't think we'd be reverting that old commit exactly, because if I remember correctly it was partly intended to make image output paths identical between meTypeset and Cermine so there wouldn't be any meaningless differences in paths between Word and PDF input

i thought this was to suit Texture. Because the editor was trying to fetch images by appending image path to the current url (see https://github.com/pkp/ots/issues/84#issuecomment-249746270)

axfelix commented 6 years ago

We are already doing that. Images are available from /manager/xml/id/:id/media/:file

Awesome, thanks, I need to document this better

w/r/t Texture stuff -- I might need to spend more time looking into this, but I have a lot of other things I need to get to today and I'll be off work for family-related travel from tomorrow through next Wednesday, so I might not have time to investigate it properly... but basically, I think we need to keep image paths in the document itself somehow relative to the document (so they just look like media/image1.png) while still supporting Texture. If you can solve that yourself, great. Otherwise, sorry I'll be away for a few days, I'll revisit when I'm back.

kaschioudi commented 6 years ago

Alright. At this point, I am waiting to hear back from Texture guys because I am stuck. Meanwhile I will replicate what I have so far to the markup plugin.