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

Fix link handling in conreffed content #3842

Merged
merged 1 commit into from
Dec 28, 2021
Merged

Conversation

jelovirt
Copy link
Member

@jelovirt jelovirt commented Dec 28, 2021

Description

Link handling in conreffed content has bugs in how target elements are selected. This PR fixes the conref stylesheet query issues.

Motivation and Context

The conrefImpl.xsl was originally written some 17 years ago with XSLT 1.0. When the conref code was moved to XSLT 2.0, the relavant code was not adjusted for the differences between XSLT 1.0 and 2.0. The code also contains a bug in selection of the first topic in a document.

How Has This Been Tested?

Existing tests pass or have been fixed to expect correct output.

Type of Changes

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

Documentation and Compatibility

Mention in release notes. Fixes a bug that's been in the codebase for 17 year. Seventeen years.

Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
@jelovirt jelovirt added bug priority/medium Medium (or unknown) priority issue preprocess/conref labels Dec 28, 2021
@jelovirt jelovirt self-assigned this Dec 28, 2021
@@ -547,7 +547,7 @@ See the accompanying LICENSE file for applicable license.
<xsl:variable name="conref-topicid" as="xs:string">
<xsl:choose>
<xsl:when test="empty($topicid)">
<xsl:value-of select="//*[contains(@class, ' topic/topic ')][1]/@id"/>
<xsl:value-of select="/descendant-or-self::*[contains(@class, ' topic/topic ')][1]/@id"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A classic XPath bug where // desugars to /descendant-or-self::node()/ and the [1] predicate no longer selects the first match of //, but rather anything that is the first child of parent node.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch

@@ -559,10 +559,10 @@ See the accompanying LICENSE file for applicable license.
<xsl:variable name="conref-gen-id" as="xs:string">
<xsl:choose>
<xsl:when test="empty($elemid) or $elemid = $href-elemid">
<xsl:value-of select="generate-id(key('id', $conref-topicid)[contains(@class, ' topic/topic ')]//*[@id = $href-elemid])"/>
<xsl:value-of select="generate-id((key('id', $conref-topicid)[contains(@class, ' topic/topic ')]//*[@id = $href-elemid])[1])"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When $conref-topicid starts to return correct values, id key will also start returning values. XSLT 1.0 will accept multiple argument values for generate-id(), but XSLT 2.0 will not. Thus we have to explicitly use only the first.

@jelovirt jelovirt merged commit e5b71bc into release/3.7 Dec 28, 2021
@jelovirt jelovirt deleted the feature/conref-cleanup branch December 28, 2021 16:01
@jelovirt jelovirt added this to the 3.7 milestone Dec 28, 2021
infotexture added a commit to dita-ot/docs that referenced this pull request Dec 29, 2021
- dita-ot/dita-ot#3842

Signed-off-by: Roger Sheen <roger@infotexture.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug preprocess/conref priority/medium Medium (or unknown) priority issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants