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

In topicpull, update "copy-shortdesc" template to process all @href values #4266

Merged
merged 1 commit into from Jan 13, 2024

Conversation

chrispy-snps
Copy link
Contributor

@chrispy-snps chrispy-snps commented Aug 14, 2023

Description

Updates topicpull processing to properly compute a topic link's description when the target topic contains a <shortdesc> containing another <xref> to a target file in a different directory.

Motivation and Context

Fixes #4244.

The code in this template has been in place for at least 13 years, as shown by the following blame link:

https://github.com/dita-ot/dita-ot/blame/49975b0660689d34178612a930a3033f47f58c83/src/main/plugins/org.dita.base/xsl/preprocess/topicpullImpl.xsl#L1251

Perhaps back then, the test for / in the @href value was because topicpull document path resolution was not as robust. It seems to work fine now. If we find a case where it doesn't, we should fix the issue instead of returning *** as the inner link text.

Once the requirement for lack of / is removed,

  • The test in the third branch of the xsl:choose simplifies to true().
    • Because empty @href values are handled by the first branch, @href is always non-empty at the third branch.
    • If we also include @href values containing /, then not(contains(@href,'/')) or contains(@href,'/') simplifies to true().
  • The fourth and fifth branches are never used.
  • The first three branches all apply the same processing, so the xsl:choose can be simplified to just that processing.

Side note - the copy-shortdesc template processing for <xref> elements is the same as the copy-shortdesc identity template:

  <xsl:template match="*|@*|processing-instruction()" mode="copy-shortdesc">
    <xsl:copy>
      <xsl:apply-templates select="@*|text()|*|processing-instruction()" mode="#current" />
    </xsl:copy>
  </xsl:template>

except that PIs are not copied.

How Has This Been Tested?

I confirmed the fix with the #4244 testcase. I also ran the unit tests, although I don't think there is currently any test coverage of this <xref>-<shortdesc>-<xref> scenario, as I can replace it with

  <xsl:template match="*[contains(@class,' topic/xref ')]" mode="copy-shortdesc">
    <xsl:message terminate="yes">FAIL</xsl:message>
  </xsl:template>

and all tests still pass.

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)

Documentation and Compatibility

No documentation update is needed.

A release notes entry could be something like:

For cross-references to a topic, the target topic’s short description is processed to create a link description. When that <shortdesc> element contained another cross-reference to a file in a different directory, the second link's description would contain *** instead of the expected link text.

Checklist

@chrispy-snps
Copy link
Contributor Author

@jelovirt - is this a good candidate for 4.1.2?

…values with "/" characters

Signed-off-by: chrispy <chrispy@synopsys.com>
@jelovirt jelovirt added bug and removed enhancement Changes to an existing feature labels Jan 13, 2024
@jelovirt jelovirt merged commit d475739 into dita-ot:develop Jan 13, 2024
3 checks passed
@jelovirt jelovirt added this to the Next milestone Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
2 participants