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

Keyref text resolution not using link text unless on link element #2014

Closed
robander opened this Issue Aug 24, 2015 · 10 comments

Comments

Projects
None yet
3 participants
@robander
Member

robander commented Aug 24, 2015

Based on clarifications to the keyref processing rules in DITA 1.3 (content was there in 1.2 but difficult to interpret):
http://docs.oasis-open.org/dita/dita/v1.3/errata01/os/complete/part1-base/archSpec/base/processing-keyref-for-text.html#processing_key_references

For any element, text can be pulled from <linktext>, which makes that a good default way to specify text with a key (number five in the list of rules). Otherwise (rule 6), normal rules apply as they would for xref, such as pulling from <navtitle>.

I've tested this with traditionally non-linking elements <cite> and <keyword>, then with <xref> and <link>. I tried keys that place text in <keyword>, <linktext>>, @navtitle, and <navtitle>. Results:

  • For non-linking elements, text is only pulled from <keyword>. If a link target is added, references to the other three will use the URI as link text.
  • For <xref>, text is pulled from the first three but not from <navtitle>. If a link target is added, the URI is used for that case.
  • For <link>, cases without a link target are removed. With a link target, results match <xref>.

Including test.ditamap and cit-test.dita:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE map PUBLIC "-//OASIS//DTD DITA Map//EN"
 "map.dtd">
<map xml:lang="en-us">
 <keydef keys="citkeyword"><topicmeta><keywords><keyword>text in keyword</keyword></keywords></topicmeta></keydef>
 <keydef keys="citlinktext"><topicmeta><linktext>text in linktext</linktext></topicmeta></keydef>
 <keydef keys="citnavatt" navtitle="navtitle attribute"/>
 <keydef keys="citnavel"><topicmeta><navtitle>navtitle element</navtitle></topicmeta></keydef>
 <keydef keys="xrefkeyword" href="http://www.dita-ot.org" format="html" scope="external">
  <topicmeta><keywords><keyword>text in keyword</keyword></keywords></topicmeta>
 </keydef>
 <keydef keys="xreflinktext" href="http://www.dita-ot.org" format="html" scope="external">
  <topicmeta><linktext>text in linktext</linktext></topicmeta>
 </keydef>
 <keydef keys="xrefnavatt" navtitle="navtitle attribute" href="http://www.dita-ot.org" format="html" scope="external"/>
 <keydef keys="xrefnavel" href="http://www.dita-ot.org" format="html" scope="external">
  <topicmeta><navtitle>navtitle element</navtitle></topicmeta>
 </keydef>

 <topicref href="cit-test.dita"/>
</map>
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE task PUBLIC "-//OASIS//DTD DITA Task//EN"
 "task.dtd">
<task id="cites" xml:lang="en-us"><?Pub Caret?>
<title>testing some citations</title>
<taskbody>
<context>
<ul>
<li><cite keyref="citkeyword"/></li>
<li><cite keyref="citlinktext"/></li>
<li><cite keyref="citnavatt"/></li>
<li><cite keyref="citnavel"/></li>
</ul>
<p>Repeat as citations that have an external link target, dita-ot.org:</p>
<ul>
<li><cite keyref="xrefkeyword"/></li>
<li><cite keyref="xreflinktext"/></li>
<li><cite keyref="xrefnavatt"/></li>
<li><cite keyref="xrefnavel"/></li>
</ul>
<p>Now try with keyword</p>
<ul>
<li><keyword keyref="citkeyword"/></li>
<li><keyword keyref="citlinktext"/></li>
<li><keyword keyref="citnavatt"/></li>
<li><keyword keyref="citnavel"/></li>
</ul>
<p>Repeat as keywords that have an external link target, dita-ot.org:</p>
<ul>
<li><keyword keyref="xrefkeyword"/></li>
<li><keyword keyref="xreflinktext"/></li>
<li><keyword keyref="xrefnavatt"/></li>
<li><keyword keyref="xrefnavel"/></li>
</ul>
<p>Repeat as cross references:</p>
<ul>
<li><xref keyref="citkeyword"/></li>
<li><xref keyref="citlinktext"/></li>
<li><xref keyref="citnavatt"/></li>
<li><xref keyref="citnavel"/></li>
</ul>
<p>Repeat as citations that have an external link target, dita-ot.org:</p>
<ul>
<li><xref keyref="xrefkeyword"/></li>
<li><xref keyref="xreflinktext"/></li>
<li><xref keyref="xrefnavatt"/></li>
<li><xref keyref="xrefnavel"/></li>
</ul>
</context>
</taskbody>
<related-links>
<link keyref="citkeyword"/>
<link keyref="citlinktext"/>
<link keyref="citnavatt"/>
<link keyref="citnavel"/>
<link keyref="xrefkeyword"/>
<link keyref="xreflinktext"/>
<link keyref="xrefnavatt"/>
<link keyref="xrefnavel"/>
</related-links>
</task>
@robander

This comment has been minimized.

Show comment
Hide comment
@robander

robander Aug 24, 2015

Member

I think this is related to both #1439 and #1590 (not sure how much overlap).

Member

robander commented Aug 24, 2015

I think this is related to both #1439 and #1590 (not sure how much overlap).

@raducoravu

This comment has been minimized.

Show comment
Hide comment
@raducoravu

raducoravu Jan 12, 2017

Member

👍 One of or end users defines keys like this:

<keydef keys="abc">
 <topicmeta>
  <linktext>ABCCONTENT</linktext>
 </topicmeta>
</keydef>

and then references them like this:

  <uicontrol keyref="abc"/>

The keyrefs are not resolved. And I agree with how @robander interprets the specs.

Member

raducoravu commented Jan 12, 2017

👍 One of or end users defines keys like this:

<keydef keys="abc">
 <topicmeta>
  <linktext>ABCCONTENT</linktext>
 </topicmeta>
</keydef>

and then references them like this:

  <uicontrol keyref="abc"/>

The keyrefs are not resolved. And I agree with how @robander interprets the specs.

@raducoravu

This comment has been minimized.

Show comment
Hide comment
@raducoravu

raducoravu Jan 12, 2017

Member

In the "org.dita.dost.writer.KeyrefPaser" class there is at some point a condition like:

                              } else if (currentElement.hasNestedElements) {
                            final NodeList linktext = elem.getElementsByTagName(TOPIC_LINKTEXT.localName);

the linktext should probably be used even if the currentElement does not have nested elements.

Member

raducoravu commented Jan 12, 2017

In the "org.dita.dost.writer.KeyrefPaser" class there is at some point a condition like:

                              } else if (currentElement.hasNestedElements) {
                            final NodeList linktext = elem.getElementsByTagName(TOPIC_LINKTEXT.localName);

the linktext should probably be used even if the currentElement does not have nested elements.

@raducoravu

This comment has been minimized.

Show comment
Hide comment
@raducoravu

raducoravu Jan 13, 2017

Member

@jelovirt, what's your opinion on by previous analysis? I think the condition should be reversed in this case, if the uicontrol is empty it's OK to use the linktext, if it's not empty use what's inside it...

Member

raducoravu commented Jan 13, 2017

@jelovirt, what's your opinion on by previous analysis? I think the condition should be reversed in this case, if the uicontrol is empty it's OK to use the linktext, if it's not empty use what's inside it...

@raducoravu

This comment has been minimized.

Show comment
Hide comment
@raducoravu

raducoravu Mar 3, 2017

Member

The same "KeyrefPaser" has an if like:

                              if (TOPIC_LINK.matches(currentElement.type)) {
                            // If the key reference element is link or its specification,
                            // should pull in the linktext

why isn't the xref treated on the same condition? Shouldn't the link text be computed for xref the same as for link?

Member

raducoravu commented Mar 3, 2017

The same "KeyrefPaser" has an if like:

                              if (TOPIC_LINK.matches(currentElement.type)) {
                            // If the key reference element is link or its specification,
                            // should pull in the linktext

why isn't the xref treated on the same condition? Shouldn't the link text be computed for xref the same as for link?

@robander robander added the P1 label Jun 2, 2017

@robander

This comment has been minimized.

Show comment
Hide comment
@robander

robander Jun 2, 2017

Member

This came up at the OASIS Lightweight DITA call today. Given the limited nature of LwDITA, whenever possible, there is only one way to do things. As of today, the only way to do phrase-level reuse is with @keyref, and the only way to define that text in a map is inside of <linktext> in the key definition. This is based on the DITA 1.3 understanding that <linktext> is the fallback that should work in all cases - text for a link, or text for [whatever element needs the text].

Unfortunately it's not working in 2.5, so I've now tagged it with P1. I'll probably be taking a look in the next week but not sure how far I'll get with resolving this issue.

Member

robander commented Jun 2, 2017

This came up at the OASIS Lightweight DITA call today. Given the limited nature of LwDITA, whenever possible, there is only one way to do things. As of today, the only way to do phrase-level reuse is with @keyref, and the only way to define that text in a map is inside of <linktext> in the key definition. This is based on the DITA 1.3 understanding that <linktext> is the fallback that should work in all cases - text for a link, or text for [whatever element needs the text].

Unfortunately it's not working in 2.5, so I've now tagged it with P1. I'll probably be taking a look in the next week but not sure how far I'll get with resolving this issue.

@raducoravu

This comment has been minimized.

Show comment
Hide comment
@raducoravu

raducoravu Jun 5, 2017

Member

The current patch I ship with the DITA OT 2.4.4 bundled with Oxygen 19.0 is that in the class "org/dita/dost/writer/KeyrefPaser" I replaced the line:

           } else if (currentElement.hasNestedElements) {

with:

        } else {

so I no longer looked if the current element (the link element) has nested elements inside it in order to use the linktext.

Member

raducoravu commented Jun 5, 2017

The current patch I ship with the DITA OT 2.4.4 bundled with Oxygen 19.0 is that in the class "org/dita/dost/writer/KeyrefPaser" I replaced the line:

           } else if (currentElement.hasNestedElements) {

with:

        } else {

so I no longer looked if the current element (the link element) has nested elements inside it in order to use the linktext.

@robander

This comment has been minimized.

Show comment
Hide comment
@robander

robander Jun 7, 2017

Member

I've been playing with the change that @raducoravu suggested and it does work for cases like this:

<topicref keydef="test">
  <topicmeta><linktext>This is my desired text</linktext></topicmeta>
</topicref>

However, it's still not working when linktext also has children - commonly needed for reuse. In the following scenario, the only text that shows up is "reuse":

<topicref keydef="test">
  <topicmeta><linktext>All about <keyword>reuse</keyword></linktext></topicmeta>
</topicref>

That's not specifically due to Radu's change - that's what we get today. I think this is based on complicated key rules that expect to use the first matching <keyword> in many scenarios, but really need the full text for this scenario.

Member

robander commented Jun 7, 2017

I've been playing with the change that @raducoravu suggested and it does work for cases like this:

<topicref keydef="test">
  <topicmeta><linktext>This is my desired text</linktext></topicmeta>
</topicref>

However, it's still not working when linktext also has children - commonly needed for reuse. In the following scenario, the only text that shows up is "reuse":

<topicref keydef="test">
  <topicmeta><linktext>All about <keyword>reuse</keyword></linktext></topicmeta>
</topicref>

That's not specifically due to Radu's change - that's what we get today. I think this is based on complicated key rules that expect to use the first matching <keyword> in many scenarios, but really need the full text for this scenario.

@robander robander self-assigned this Jun 7, 2017

@robander

This comment has been minimized.

Show comment
Hide comment
@robander

robander Jun 8, 2017

Member

I tugged on this thread on the code and found lots of other little issues. Will have a pull request up soon with Radu's original suggestion (which fixes the general plain-text <linktext> condition) and some other special conditions.

Member

robander commented Jun 8, 2017

I tugged on this thread on the code and found lots of other little issues. Will have a pull request up soon with Radu's original suggestion (which fixes the general plain-text <linktext> condition) and some other special conditions.

jelovirt added a commit that referenced this issue Jun 20, 2017

Fix key resolution text #2014 and more (#2719)
* Update fallback text resolution for key reference elements

Signed-off-by: Robert D Anderson <robander@us.ibm.com>

* Correct test case for xref with two keywords

Signed-off-by: Robert D Anderson <robander@us.ibm.com>

* Extend fallback to use href if nothing else found

Signed-off-by: Robert D Anderson <robander@us.ibm.com>

* Update test cases for href fallback when no other key text

Signed-off-by: Robert D Anderson <robander@us.ibm.com>

* New test cases for keyref text resolution

Signed-off-by: Robert D Anderson <robander@us.ibm.com>

* Use streams to process collections and extract code to separate methods

@robander robander added this to the 2.5.1 milestone Jun 20, 2017

@robander

This comment has been minimized.

Show comment
Hide comment
@robander

robander Jun 20, 2017

Member

Fixed with #2719

Member

robander commented Jun 20, 2017

Fixed with #2719

@robander robander closed this Jun 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment