TOC Links Broken If Also Module Names #597

Closed
Smylers opened this Issue Jun 6, 2012 · 9 comments

Comments

Projects
None yet
3 participants
@Smylers
Contributor

Smylers commented Jun 6, 2012

If an item in a module's table of contents happens to also be the name of another module then the link in the TOC goes to the other module's page, rather than to the identically named section on the current page.

For example see the perl 5.17.0 perldelta. The entry ‘Documentation’ links to #Documentation, but its sub-entry ‘perldata’ links to the perldata page, not to the #perldata, the perldata sub-section on that page.

@benkasminbullock

This comment has been minimized.

Show comment Hide comment
@benkasminbullock

benkasminbullock Jun 6, 2012

Contributor

@Smylers

If an item in a module's table of contents happens to also be the name of another module then the link in the TOC goes to the other module's page, rather than to the identically named section on the current page.

Are you sure? Do you have any other examples? The document in question,

http://api.metacpan.org/source/ZEFRAM/perl-5.17.0/pod/perldelta.pod

actually uses

=head3 L<perldata>

for its heading, which is meant to turn into a link to the perldelta page. That is kind of an "error" or at least a bizarre bit of behaviour in the original document, which might be needed to code around, but it's not a bug in metacpan.

Contributor

benkasminbullock commented Jun 6, 2012

@Smylers

If an item in a module's table of contents happens to also be the name of another module then the link in the TOC goes to the other module's page, rather than to the identically named section on the current page.

Are you sure? Do you have any other examples? The document in question,

http://api.metacpan.org/source/ZEFRAM/perl-5.17.0/pod/perldelta.pod

actually uses

=head3 L<perldata>

for its heading, which is meant to turn into a link to the perldelta page. That is kind of an "error" or at least a bizarre bit of behaviour in the original document, which might be needed to code around, but it's not a bug in metacpan.

@Smylers

This comment has been minimized.

Show comment Hide comment
@Smylers

Smylers Jun 7, 2012

Contributor

Ben Bullock writes:

The document in question,

http://api.metacpan.org/source/ZEFRAM/perl-5.17.0/pod/perldelta.pod

actually uses

=head3 L

for its heading,

Ah, I see. My apologies for mis-diagnosing this as a more generic
problem, and for not doing more research first.

which is meant to turn into a link to the perldelta page.

Yup, and indeed the heading does link to that page as intended, which is
good.

That is kind of an "error" or at least a bizarre bit of behaviour in
the original document, which might be needed to code around,

I don't see any problem with it; it's a useful link to have, and the
heading itself is rendered just fine, as:

perldata

but it's not a bug in metacpan.

I think it is a bug, because it's the table of contents (which is only
added for the HTML display) which has the problem, displaying a list of
section headings for that page but having some of them linking
elsewhere:
https://metacpan.org/module/ZEFRAM/perl-5.17.0/pod/perldelta.pod

Looking at the HTML source, the HTML for those TOC entries isn't valid,
because it has nested hyperlinks:

  • perldata
  • So I think what's needed is simply to remove any existing links (but not
    the link text, of course) from TOC entries before wrapping them with the
    TOC section link.

    Cheers, and sorry again for my mistake.

    Smylers

    http://twitter.com/Smylers2

    Contributor

    Smylers commented Jun 7, 2012

    Ben Bullock writes:

    The document in question,

    http://api.metacpan.org/source/ZEFRAM/perl-5.17.0/pod/perldelta.pod

    actually uses

    =head3 L

    for its heading,

    Ah, I see. My apologies for mis-diagnosing this as a more generic
    problem, and for not doing more research first.

    which is meant to turn into a link to the perldelta page.

    Yup, and indeed the heading does link to that page as intended, which is
    good.

    That is kind of an "error" or at least a bizarre bit of behaviour in
    the original document, which might be needed to code around,

    I don't see any problem with it; it's a useful link to have, and the
    heading itself is rendered just fine, as:

    perldata

    but it's not a bug in metacpan.

    I think it is a bug, because it's the table of contents (which is only
    added for the HTML display) which has the problem, displaying a list of
    section headings for that page but having some of them linking
    elsewhere:
    https://metacpan.org/module/ZEFRAM/perl-5.17.0/pod/perldelta.pod

    Looking at the HTML source, the HTML for those TOC entries isn't valid,
    because it has nested hyperlinks:

  • perldata
  • So I think what's needed is simply to remove any existing links (but not
    the link text, of course) from TOC entries before wrapping them with the
    TOC section link.

    Cheers, and sorry again for my mistake.

    Smylers

    http://twitter.com/Smylers2

    @benkasminbullock

    This comment has been minimized.

    Show comment Hide comment
    @benkasminbullock

    benkasminbullock Jun 7, 2012

    Contributor
    I think it is a bug, because it's the table of contents (which is only added for the HTML display) which has the problem, displaying a list of section headings for that page but having some of them linking elsewhere:

    I agree it's sort of a bug but the pod itself is the problem.

    Contributor

    benkasminbullock commented Jun 7, 2012

    I think it is a bug, because it's the table of contents (which is only added for the HTML display) which has the problem, displaying a list of section headings for that page but having some of them linking elsewhere:

    I agree it's sort of a bug but the pod itself is the problem.

    @Smylers

    This comment has been minimized.

    Show comment Hide comment
    @Smylers

    Smylers Jun 7, 2012

    Contributor

    Ben Bullock writes:

    I agree it's sort of a bug

    The TOC has invalid HTML, so that seems unambiguously a bug in the TOC
    generation.

    but the pod itself is the problem.

    In what way?

    It passes podchecker, and the heading itself renders fine on the page,
    complete with the intended link to the page that the section is talking
    about.

    So far as I can tell the only problem is with the TOC, which isn't in
    the Pod.

    Cheers

    Smylers

    http://twitter.com/Smylers2

    Contributor

    Smylers commented Jun 7, 2012

    Ben Bullock writes:

    I agree it's sort of a bug

    The TOC has invalid HTML, so that seems unambiguously a bug in the TOC
    generation.

    but the pod itself is the problem.

    In what way?

    It passes podchecker, and the heading itself renders fine on the page,
    complete with the intended link to the page that the section is talking
    about.

    So far as I can tell the only problem is with the TOC, which isn't in
    the Pod.

    Cheers

    Smylers

    http://twitter.com/Smylers2

    @benkasminbullock

    This comment has been minimized.

    Show comment Hide comment
    @benkasminbullock

    benkasminbullock Jun 7, 2012

    Contributor

    This bug originates from the API part of MetaCPAN rather than the web site. It originates from a bug in Pod::Simple::XHTML, as can be seen from the following script:

    #!/home/ben/software/install/bin/perl
    use warnings;
    use strict;
    use File::Slurp;
    use Pod::Simple::XHTML;
    my $c = read_file ('perldelta.pod');
    my $d = Pod::Simple::XHTML->new ();
    $d->index (1);
    my $e;
    $d->output_string (\$e);
    $d->parse_string_document ($c);
    print $e;
    

    This is a partial copy of what is in cpan-api/lib/MetaCPAN/Server/View/Pod.pm.

    I have reported the bug upstream to the Pod::Simple::XHTML bug system:

    https://rt.cpan.org/Ticket/Display.html?id=77686

    Contributor

    benkasminbullock commented Jun 7, 2012

    This bug originates from the API part of MetaCPAN rather than the web site. It originates from a bug in Pod::Simple::XHTML, as can be seen from the following script:

    #!/home/ben/software/install/bin/perl
    use warnings;
    use strict;
    use File::Slurp;
    use Pod::Simple::XHTML;
    my $c = read_file ('perldelta.pod');
    my $d = Pod::Simple::XHTML->new ();
    $d->index (1);
    my $e;
    $d->output_string (\$e);
    $d->parse_string_document ($c);
    print $e;
    

    This is a partial copy of what is in cpan-api/lib/MetaCPAN/Server/View/Pod.pm.

    I have reported the bug upstream to the Pod::Simple::XHTML bug system:

    https://rt.cpan.org/Ticket/Display.html?id=77686

    @Smylers

    This comment has been minimized.

    Show comment Hide comment
    @Smylers

    Smylers Jun 7, 2012

    Contributor

    I have reported the bug upstream to the Pod::Simple::XHTML bug system:

    Thanks.

    Contributor

    Smylers commented Jun 7, 2012

    I have reported the bug upstream to the Pod::Simple::XHTML bug system:

    Thanks.

    @benkasminbullock

    This comment has been minimized.

    Show comment Hide comment
    @benkasminbullock

    benkasminbullock Jun 11, 2012

    Contributor

    This is now fixed upstream by @theory by commit 29306b765cc859f139655c34528e9b510a0452d9 on pod-simple. Once a new version of Pod::Simple is released to cpan it can be installed and hopefully this issue will be fixed.

    Contributor

    benkasminbullock commented Jun 11, 2012

    This is now fixed upstream by @theory by commit 29306b765cc859f139655c34528e9b510a0452d9 on pod-simple. Once a new version of Pod::Simple is released to cpan it can be installed and hopefully this issue will be fixed.

    @ghost ghost assigned monken Jul 31, 2012

    @monken

    This comment has been minimized.

    Show comment Hide comment
    @monken

    monken Jul 31, 2012

    Member

    Note to self: bump prereq in cpan-api while I'm at it
    Depends on https://github.com/theory/pod-simple/issues/33

    Member

    monken commented Jul 31, 2012

    Note to self: bump prereq in cpan-api while I'm at it
    Depends on https://github.com/theory/pod-simple/issues/33

    Smylers added a commit to Smylers/cpan-api that referenced this issue Oct 8, 2012

    Pod::Simple::XHTML minimum version
    Ensure Pod::Simple::XHTML is new enough to avoid the problem reported in metacpan/metacpan-web#597

    @Smylers Smylers referenced this issue in metacpan/metacpan-api Oct 8, 2012

    Merged

    Pod::Simple::XHTML minimum version #236

    @Smylers

    This comment has been minimized.

    Show comment Hide comment
    @Smylers

    Smylers Oct 8, 2012

    Contributor

    Fix is now live and working fine, so closing this report.

    monken's note to self is now a pull request on cpan-api: CPAN-API/cpan-api#236

    Contributor

    Smylers commented Oct 8, 2012

    Fix is now live and working fine, so closing this report.

    monken's note to self is now a pull request on cpan-api: CPAN-API/cpan-api#236

    @Smylers Smylers closed this Oct 8, 2012

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