Allow text-only links to appear with related-links #3030
Conversation
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
robander
added
plugin/pdf
robander
changed the title
| @@ -340,7 +340,7 @@ See the accompanying LICENSE file for applicable license. | |||
| </xsl:variable> | |||
|
|
|||
| <xsl:for-each select="ancestor::*[contains(@class, ' map/relrow ')]/*[contains(@class, ' map/relcell ')][position()!=$position]"> | |||
| <xsl:if test="descendant::*[contains(@class, ' map/topicref ')][@href and not(@href = '')][not(@linking = ('none', 'sourceonly'))]"> | |||
| <xsl:if test="descendant::*[contains(@class, ' map/topicref ')][(@href and not(@href = '')) or */*[contains(@class,' map/linktext ') or contains(@class,' topic/linktext ')]][not(@linking = ('none', 'sourceonly'))]"> | |||
There was a problem hiding this comment.
The added set of predicate conditions are repeated in multiple placed in this stylesheet. IMO this should be extracted into a function with a descriptive name to make the code easier to read and understand.
There was a problem hiding this comment.
Made the [(href and not href='') or linktext] condition into a function.
Still need to add an integration test for this to ensure the links are updated properly by maplink + not dropped by topicpull.
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
| @@ -489,7 +489,7 @@ See the accompanying LICENSE file for applicable license. | |||
| </xsl:choose> | |||
| </xsl:template> | |||
|
|
|||
| <xsl:template match="*[contains(@class,' topic/link ')]" mode="processLink"> | |||
| <xsl:template match="*[contains(@class,' topic/link ')][not(empty(@href) or @href='')]" mode="processLink"> | |||
There was a problem hiding this comment.
I would like us to at some point consider adding empty @href removal to preprocessing. Then we would not have to have @href = '' tests in transtype specific code. Or is there any reason why to use an empty @href attribute? URI.create("file:/base/topic.xml").resolve("") will resolve to file:/base/, but I don't see any reason to allow users to link to directories. If we remove empty @href, you could still link to those with href="./".
There was a problem hiding this comment.
👍 for removing empty @href (and empty @conref) early in initial processing. It's caused problems several times over the year when other steps don't test for it, and try to open the directory -- exactly as you've described. For an author running a build it's extremely confusing and hard to track down, because it usually appears as "Cannot open /base/" in the error. So yeah -- leaving it in means a lot of messy checks in the code, and means if you forget the check, things go wrong.
The only reason I can think of for having it is if you have some element with required @href attribute, but you don't actually want to specify one -- maybe because you use @conref or @keyref. Technically that's what the -dita-use-conref-target token is for but most people don't know about that. Even for that case though -- the empty @href would only be needed for initial parsing / validation, and generic DITA processing code that follows won't look for it.
Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
Signed-off-by: Jarno Elovirta <jarno@elovirta.com>
Description
Includes several related fixes / enhancements in order to implement #1132:
topicpullremoves any specified<linktext>or<desc>content when there is no link target, resulting in a truly empty link. This should be deferred to rendering to determine whether / how to render links without@href.@hrefdoes make its way to PDF processing today, it results in a build failure. The build failure has been removed and the text is rendered with other links.<topicref> that does not have a link target, but _does_ include`), those links are generated, and the final rendering step is allowed to render or hide them as needed.<xref>does not specify any@hrefvalue at all, remove the error message that complains about an empty@hrefvalue<link>does not specify@href, avoid generating a build error intopicpull, and leave this to rendering steps or customizations to determine if it is an error.Motivation and Context
Reported / requested long ago with #1132, request is still valid and outstanding.
How Has This Been Tested?
Test files attached in the original issue to reproduce all issues.
Also encountered #3029 while testing this issue, but that is a more general issue that should be fixed ASAP in a hotfix rather than bundled with this enhancement.
Type of Changes