Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.3
    • Fix Version/s: 1.4
    • Labels:
      None
    • Number of attachments :
      1

      Description

      Using a link to a javadoc method like

      {{{../apidocs/groovyx/net/http/ParserRegistry.html#parseText(org.apache.http.HttpResponse)}ParserRegistry}}
      

      the apt parser removes the brackets of the anchor. The same thing happens with xdocs and probably other formats. Note that non-ascii characters are not legal in anchor names, but they should be replaced by their hex values, see http://www.w3.org/TR/html401/appendix/notes.html#non-ascii-chars.

        Issue Links

          Activity

          Hide
          Thom Nichols added a comment -

          It also removes commas (which are unescaped in JavaDoc links) and already-escaped octets lose the '%'. So pre-escaping non-alphanumeric characters doesn't help in this case.

          So

          {{{../apidocs/groovyx/net/http/HTTPBuilder.html#get%28java.util.Map,%20groovy.lang.Closure%29}get()}}>>> 
          

          becomes

          <a href='../apidocs/groovyx/net/http/HTTPBuilder.html#get28java.util.Map20groovy.lang.Closure29'>get()</a> 
          
          Show
          Thom Nichols added a comment - It also removes commas (which are unescaped in JavaDoc links) and already-escaped octets lose the '%'. So pre-escaping non-alphanumeric characters doesn't help in this case. So {{{../apidocs/groovyx/net/http/HTTPBuilder.html#get%28java.util.Map,%20groovy.lang.Closure%29}get()}}>>> becomes <a href='../apidocs/groovyx/net/http/HTTPBuilder.html#get28java.util.Map20groovy.lang.Closure29'>get()</a>
          Hide
          Lukas Theussl added a comment -

          I was about to commit a fix when I saw your example above and realized that it won't work for more than one argument. The problem is the space after the comma, which doxia replaces by an underscore, instead of its hex value (http://maven.apache.org/doxia/doxia/doxia-core/apidocs/org/apache/maven/doxia/util/DoxiaUtils.html#encodeId%28java.lang.String,%20boolean%29 ).

          The reason for this choice of replacement, I think, was to generate readable links for the most typical use-case of anchors in apt documents, eg links to section titles, etc. I'm afraid we will break a lot of stuff if we change this now.

          Show
          Lukas Theussl added a comment - I was about to commit a fix when I saw your example above and realized that it won't work for more than one argument. The problem is the space after the comma, which doxia replaces by an underscore, instead of its hex value ( http://maven.apache.org/doxia/doxia/doxia-core/apidocs/org/apache/maven/doxia/util/DoxiaUtils.html#encodeId%28java.lang.String,%20boolean%29 ). The reason for this choice of replacement, I think, was to generate readable links for the most typical use-case of anchors in apt documents, eg links to section titles, etc. I'm afraid we will break a lot of stuff if we change this now.
          Hide
          Thom Nichols added a comment -

          So the behavior, if I understand correctly, relates to what's described here: http://maven.apache.org/doxia/references/doxia-apt.html#Links
          Specifically, Doxia is parsing 'local' links as if they are 'anchors' (or 'internal links'?)

          If Doxia sees a link that starts with './' or '../', should it be manipulating it at all? I thought a local link had to be a valid URI fragment?

          Show
          Thom Nichols added a comment - So the behavior, if I understand correctly, relates to what's described here: http://maven.apache.org/doxia/references/doxia-apt.html#Links Specifically, Doxia is parsing 'local' links as if they are 'anchors' (or 'internal links'?) If Doxia sees a link that starts with './' or '../', should it be manipulating it at all? I thought a local link had to be a valid URI fragment?
          Hide
          Lukas Theussl added a comment - - edited

          Not really, this issue has nothing to do with local, internal or external links. These notions only distinguish the case of where the link points to (target within same source document, target within same site, target outside), but in each case the URI has to be valid. Since Doxia 1.1 we fix all invalid ids, that's why the warning is emitted, you are supposed to correct the links in your source documents.

          Btw, it should work if you use absolute links, as only local and internal links are re-written. Eg if you link from one apt document to another within your site, you can control both the link (href) and the target (anchor), but if you link to the internet, you usually do not control the target. Unfortunately, javadoc generates invalid anchors, which is apparently a well-known fact: http://mindprod.com/jgloss/javadoc.html#BUGS

          Show
          Lukas Theussl added a comment - - edited Not really, this issue has nothing to do with local, internal or external links. These notions only distinguish the case of where the link points to (target within same source document, target within same site, target outside), but in each case the URI has to be valid. Since Doxia 1.1 we fix all invalid ids, that's why the warning is emitted, you are supposed to correct the links in your source documents. Btw, it should work if you use absolute links, as only local and internal links are re-written. Eg if you link from one apt document to another within your site, you can control both the link (href) and the target (anchor), but if you link to the internet, you usually do not control the target. Unfortunately, javadoc generates invalid anchors, which is apparently a well-known fact: http://mindprod.com/jgloss/javadoc.html#BUGS
          Hide
          Thom Nichols added a comment -

          So you can have a link to a 'local' Doxia ID that's not on the same page? I assumed local links had to be valid relative URIs.

          It sounds like the proper solution would be to have two link formats - one for URIs and one for Doxia IDs. Otherwise, how can Doxia tell if this:

           {{{../apidocs/groovyx/net/http/HTTPBuilder.html#defaultSuccessHandler%28org.apache.http.HttpResponse%2C%20java.lang.Object%29}defaultSuccessHandler}}
          

          is a Doxia ID or a relative URI?

          Show
          Thom Nichols added a comment - So you can have a link to a 'local' Doxia ID that's not on the same page? I assumed local links had to be valid relative URIs. It sounds like the proper solution would be to have two link formats - one for URIs and one for Doxia IDs. Otherwise, how can Doxia tell if this: {{{../apidocs/groovyx/net/http/HTTPBuilder.html#defaultSuccessHandler%28org.apache.http.HttpResponse%2C%20java.lang. Object %29}defaultSuccessHandler}} is a Doxia ID or a relative URI?
          Hide
          Thom Nichols added a comment -

          Or, if it's not a valid Doxia ID, just complain (emit a warning message) but don't munge it.

          But maybe some people are relying on behavior to turn {{

          {Some Section Heading}

          blah}} into <a href='#Some_Section_Heading'>blah</a>

          Show
          Thom Nichols added a comment - Or, if it's not a valid Doxia ID, just complain (emit a warning message) but don't munge it. But maybe some people are relying on behavior to turn {{ {Some Section Heading} blah}} into <a href='#Some_Section_Heading'>blah</a>
          Hide
          Lukas Theussl added a comment -

          A link like

          {{{../apidocs/groovyx/.....}}}
          

          is not a Doxia id because it contains slashes.

          just complain (emit a warning message) but don't munge it.

          It is assumed that site builders have control over both target ids and links for internal and local links, ie if Doxia fixes an invalid id, the developer should be forced to fix the link and avoid the warning. Unfortunately this is not possible for auto-generated docs that produce invalid anchors/links, like javadoc.

          But maybe some people are relying on behavior to turn {Some Section Heading}blah into <a href='#Some_Section_Heading'>blah</a>

          Yes, that's the origin of the problem, we would break a lot of existing links if we changed that now.

          Show
          Lukas Theussl added a comment - A link like {{{../apidocs/groovyx/.....}}} is not a Doxia id because it contains slashes. just complain (emit a warning message) but don't munge it. It is assumed that site builders have control over both target ids and links for internal and local links, ie if Doxia fixes an invalid id, the developer should be forced to fix the link and avoid the warning. Unfortunately this is not possible for auto-generated docs that produce invalid anchors/links, like javadoc. But maybe some people are relying on behavior to turn {Some Section Heading}blah into <a href='#Some_Section_Heading'>blah</a> Yes, that's the origin of the problem, we would break a lot of existing links if we changed that now.
          Hide
          Thom Nichols added a comment -

          It is assumed that site builders have control over both target ids and links for internal and local links, ie if Doxia fixes an invalid id, the developer should be forced to fix the link and avoid the warning.

          But the developer isn't being forced to fix the link, Doxia is attempting to fix it for me (and breaking it in the process.) I think you mean, if Doxia fixes the link, the developer will be forced to fix the anchor that it points to?

          Unfortunately this is not possible for auto-generated docs that produce invalid anchors/links, like javadoc.

          But even if I properly escape the link, Doxia still munges it. I don't think this has anything to do with the fact that Javadoc leaves unescaped characters in the URI. A valid URI like:

          {{{../apidocs/groovyx/net/http/HTTPBuilder.html#defaultSuccessHandler%28org.apache.http.HttpResponse%2C%20java.lang.Object%29}defaultSuccessHandler}}
          

          is still getting smashed by Doxia.

          I apologize for being dense, as I feel like keep asking the same question. But the above link – you said it cannot be a Doxia ID. OK! So if it's not a Doxia ID, then it must be a relative URI, right? So if it's a relative URI, why would Doxia ever try to manipulate it? And if it does feel the need to manipulate a "non Doxia ID" link, shouldn't it just be doing normal URI escaping like replacing " " with "%20"? But instead it appears that the "Doxia ID" rules (converting spaces to underscores and removing characters like '%') are being applied to links that are clearly relative URIs. I guess I fail to understand what the intended goal is in this case.

          Can't the logic be "If I see what appears to be a Doxia ID, perform the Doxia link/anchor manipulation." "If I see a link that's not a Doxia ID, just apply URI-escaping rules."

          A link like "{{

          {Some Section Heading}

          blah}}" is clearly a Doxia ID, so don't change any of the current behavior in that case and preserve backward compatibility.

          Show
          Thom Nichols added a comment - It is assumed that site builders have control over both target ids and links for internal and local links, ie if Doxia fixes an invalid id, the developer should be forced to fix the link and avoid the warning. But the developer isn't being forced to fix the link, Doxia is attempting to fix it for me (and breaking it in the process.) I think you mean, if Doxia fixes the link, the developer will be forced to fix the anchor that it points to? Unfortunately this is not possible for auto-generated docs that produce invalid anchors/links, like javadoc. But even if I properly escape the link, Doxia still munges it. I don't think this has anything to do with the fact that Javadoc leaves unescaped characters in the URI. A valid URI like: {{{../apidocs/groovyx/net/http/HTTPBuilder.html#defaultSuccessHandler%28org.apache.http.HttpResponse%2C%20java.lang.Object%29}defaultSuccessHandler}} is still getting smashed by Doxia. I apologize for being dense, as I feel like keep asking the same question. But the above link – you said it cannot be a Doxia ID. OK! So if it's not a Doxia ID, then it must be a relative URI, right? So if it's a relative URI, why would Doxia ever try to manipulate it? And if it does feel the need to manipulate a "non Doxia ID" link, shouldn't it just be doing normal URI escaping like replacing " " with "%20"? But instead it appears that the "Doxia ID" rules (converting spaces to underscores and removing characters like '%') are being applied to links that are clearly relative URIs. I guess I fail to understand what the intended goal is in this case. Can't the logic be "If I see what appears to be a Doxia ID, perform the Doxia link/anchor manipulation." "If I see a link that's not a Doxia ID, just apply URI-escaping rules." A link like "{{ {Some Section Heading} blah}}" is clearly a Doxia ID, so don't change any of the current behavior in that case and preserve backward compatibility.
          Hide
          Thom Nichols added a comment -

          I've fixed this myself, the patch is trivial and I don't believe it breaks backwards compatibility. Please try it out and let me know if it causes any problems with existing apt sources.

          Show
          Thom Nichols added a comment - I've fixed this myself, the patch is trivial and I don't believe it breaks backwards compatibility. Please try it out and let me know if it causes any problems with existing apt sources.
          Hide
          Michael Osipov added a comment -

          Is there any update on this? Just ran into this annoying bug.

          Show
          Michael Osipov added a comment - Is there any update on this? Just ran into this annoying bug.
          Hide
          Lukas Theussl added a comment -

          I have added a test case that gets broken by the attached patch. I think this is a rather common use case, as mentioned above, fixing this will break a lot of stuff (note that apt does not distinguish anchor names from anchor text, ie something like <a name="name">text</a> is not possible in apt, so there are many examples of anchors like

          {anchor with spaces}

          ).

          I agree it's annoying, but I'd prefer to postpone a fix for this until a major version change. Until then I suggest to use abolute links for javadocs.

          Show
          Lukas Theussl added a comment - I have added a test case that gets broken by the attached patch. I think this is a rather common use case, as mentioned above, fixing this will break a lot of stuff (note that apt does not distinguish anchor names from anchor text, ie something like <a name="name">text</a> is not possible in apt, so there are many examples of anchors like {anchor with spaces} ). I agree it's annoying, but I'd prefer to postpone a fix for this until a major version change. Until then I suggest to use abolute links for javadocs.
          Hide
          vincent laugier added a comment -

          I am currently using doxia apt to generate a maven website (with maven-site-plugin:3.0-beta-3)

          The problem is still the same:

          [WARNING] [APT Parser] Modified invalid link: 'initializeGrid(org.primefaces.component.panelgrid.PanelGrid)' to './apidocs/org/objectwiz/web/backingbean/EjbClientBackingBean.html#initializeGridorg.primefaces.component.panelgrid.PanelGrid'

          I guess that there is nothing to be done (the bug would have been fixed by now).

          How can we include html into a apt document ?

          Show
          vincent laugier added a comment - I am currently using doxia apt to generate a maven website (with maven-site-plugin:3.0-beta-3) The problem is still the same: [WARNING] [APT Parser] Modified invalid link: 'initializeGrid(org.primefaces.component.panelgrid.PanelGrid)' to './apidocs/org/objectwiz/web/backingbean/EjbClientBackingBean.html#initializeGridorg.primefaces.component.panelgrid.PanelGrid' I guess that there is nothing to be done (the bug would have been fixed by now). How can we include html into a apt document ?
          Hide
          Michael Osipov added a comment -

          Vicent, unless fix doxia yourself the problem will persist. The problem is that the standard JavaDoc doclet generates invalid anchors and doxia rejects them.

          Show
          Michael Osipov added a comment - Vicent, unless fix doxia yourself the problem will persist. The problem is that the standard JavaDoc doclet generates invalid anchors and doxia rejects them.
          Hide
          vincent laugier added a comment -

          Hello Michael,

          I have eventually decided to use some absolute links

          and filter my apt files for convenience

          http://maven.apache.org/plugins/maven-resources-plugin/examples/filter.html

          Thanks

          Show
          vincent laugier added a comment - Hello Michael, I have eventually decided to use some absolute links and filter my apt files for convenience http://maven.apache.org/plugins/maven-resources-plugin/examples/filter.html Thanks
          Hide
          Michael Osipov added a comment -

          Have you tried to use URI escapes?

          Show
          Michael Osipov added a comment - Have you tried to use URI escapes?
          Hide
          Robert Scholte added a comment -

          Fixed in r1465234
          A link with a single '#' will be translated to a Doxia-ID.
          A link with '##' (which would normally be illegal) will copy the anchor as is. This means that the developer is responsible for the correct URL encoding!

          Show
          Robert Scholte added a comment - Fixed in r1465234 A link with a single '#' will be translated to a Doxia-ID. A link with '##' (which would normally be illegal) will copy the anchor as is. This means that the developer is responsible for the correct URL encoding!

            People

            • Assignee:
              Robert Scholte
              Reporter:
              Lukas Theussl
            • Votes:
              7 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: