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

Handle relative paths in peer or non-DITA key references #3234

Merged
merged 2 commits into from Feb 25, 2019

Conversation

Projects
3 participants
@robander
Copy link
Member

commented Feb 21, 2019

Fixes #2620, #2581, #2250, and #1951 (recently closed by probot).

Currently, when a key definition uses a relative path, references to that key are evaluated as follows:

  • If the key reference is on <image>, adjust relative path based on location of the key definition / key references
  • Else if it is for a local DITA topic, adjust relative path based on location of the key definition / key references
  • Else use the path as-is. If the key is defined with a relative path, but referenced from a different directory, this breaks all of the following cases (as reported in the various issues above):
  • Keys for peer DITA topics like ../peer/some.dita, when referenced from a directory different from the map, are no longer valid
  • Keys for local non-DITA file, such as example.pdf, are also not adjusted based on the directory
  • This has been reported several times for @datakeyref on <object>, which will break if the key definition refers to a local file but the key reference is in a different directory
  • If images are referenced by link (rather than from <image>), the same issue comes up, because the special handling for images only works on <image> element.

This fix corrects all of those cases by testing the @href value on the key definition:

  • If the path is absolute, OR if scope="external", leave it as-is (no change from today)
  • Otherwise, it is a relative path and either local or peer, so adjust the path as needed to stay valid in any referencing location

Also updated keyref unit tests to cover all of these conditions, to ensure that we test for keys defined in one directory but referenced from a topic in a different directory.

robander added some commits Feb 21, 2019

Adjust relpath for peer or non-dita keys #2620 #2581 #2250
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Test for conditions in #2620 #2581 #2250 #1951
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

@robander robander added this to In progress in 3.3 via automation Feb 21, 2019

@robander robander moved this from In progress to Needs review in 3.3 Feb 21, 2019

@robander robander requested a review from jelovirt Feb 21, 2019

@raducoravu

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Fix looks good to me, I tested and it works for HTML5 output. I have a problem with the PDF output, it may not be related to this issue but a folder called "${_dita.map.output.dir}" gets created in the working directory (place where the Java process starts) and it contains the video inside it.
Also for the PDF output I looked inside the _MERGED.xml file and it contains this reference:

      <object datakeyref="shining_dots" outputclass="video" class="- topic/object " xtrf="file:/C:/Users/radu_coravu/Desktop/datakeyrefProblem/space-multimedia/topics/movies.dita" xtrc="object:1;20:15" data="99dc317f2c89406f64f6ccb34a8be40f53de63cc.mp4" format="mp4"/>

But that "99dc317f2c89406f64f6ccb34a8be40f53de63cc.mp4" file is not present next to the merged file.
Of course for the PDF output the DITA object element makes little sense, ideally it could get converted to a plain link to the video in this case.

3.3 automation moved this from Needs review to Reviewer approved Feb 23, 2019

@robander

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2019

@raducoravu probably best to open a new issue so that doesn't get lost -- I'm guessing (?) the output today isn't working right for PDF anyway, so if anything that is just not-working-in-a-different-way?

@robander robander merged commit 93efd7e into develop Feb 25, 2019

4 checks passed

DCO DCO
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

3.3 automation moved this from Reviewer approved to Done Feb 25, 2019

@robander robander added this to the 3.3 milestone Feb 25, 2019

@raducoravu

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

@robander I added a new issue with the PDF side-effect here: #3242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.