Off-the-shelf XSLT doesn't always show labels for nested properties

rwalkerands commented 9 years ago

The off-the-shelf XSLT stylesheet ashtml.xsl doesn't always display labels for nested properties.

I'm using this viewer:

andsvocabs:basicConceptViewerNarrowerBroader a api:Viewer
    ; api:name "concept-labelled"
    ; api:include api:labelledDescribeViewer
    ; api:property skos:prefLabel, skos:notation,
      ( skos:broader skos:prefLabel ), skos:definition,
      ( skos:narrower skos:prefLabel )
    .

Here are samples for two vocabularies, with identical endpoint definitions and both using the unmodified ashtml.xsl stylesheet:

Page (1) displays correctly. The broader and narrower concepts have their prefLabels displayed correctly. Page (2) does not display correctly. The broader and narrower concepts show only the last component of the resource IRI (i.e., one or two digits).

The different behaviour seems to be provoked by differences in the underlying XML that gets fed in to the XSLT. I implemented a pass-through XSLT formatter so I can see this XML.

Compare:

In page (3), each narrower/broader element has an item element, which in turn has a prefLabel subelement. In page (4), each broader/narrower element has an an item element only; no prefLabel subelement.

The reason seems to be the method com.epimorphics.lda.renderers.XMLRendering.elementAddResource(), which calls Trail.expand() to determine whether or not to expand the node further. The test blocked.contains(x) returns true, so expand() returns false, and the node is not expanded further.

In page (3), it so happens that the broader and narrower terms do not otherwise appear as top-level resources in the results (i.e., they were not resources selected for viewing), so they get expanded to the next level. In page (4), it happens that the broader and narrower terms do also appear as top-level resources in the results, so they don't get expanded further when they appear as narrower/broader concepts.

My supposition is that this non-expansion in the XML is all deliberate, and done in order to prevent an infinite recursion of expansion of nodes. In which case, the XSLT formatter ashtml.xsl needs to be modified to cope.

rwalkerands commented 9 years ago

Any indication that someone's going to have a look at this?

rwalkerands commented 9 years ago

My local XSLT guru has drafted a fix.

Try updating this template:

<xsl:template match="*[@href]" mode="content">
        <xsl:param name="nested" select="false()" />
        <xsl:choose>
                <xsl:when test="$nested">
                        <xsl:apply-templates select="." mode="table" />
                </xsl:when>
                <xsl:otherwise>
                        <xsl:apply-templates select="." mode="link">
                                <xsl:with-param name="content">
                                        <xsl:call-template name="lastURIpart">
                                                <xsl:with-param name="uri" select="@href" />
                                        </xsl:call-template>
                                </xsl:with-param>
                        </xsl:apply-templates>
                </xsl:otherwise>
        </xsl:choose>
</xsl:template>

to read like this:

<xsl:template match="*[@href]" mode="content">
        <xsl:param name="nested" select="false()"/>
        <xsl:choose>
                <xsl:when test="$nested">
                        <xsl:apply-templates select="." mode="table"/>
                </xsl:when>
                <xsl:otherwise>
                        <xsl:choose>
                                <xsl:when test="/result/items/item[@href = current()/@href]">
                                        <xsl:apply-templates select="/result/items/item[@href = current()/@href]" mode="link">
                                                <xsl:with-param name="content">
                                                        <xsl:apply-templates select="/result/items/item[@href = current()/@href]" mode="name"/>
                                                </xsl:with-param>
                                        </xsl:apply-templates>
                                </xsl:when>
                                <xsl:otherwise>
                                        <xsl:apply-templates select="." mode="link">
                                                <xsl:with-param name="content">
                                                        <xsl:call-template name="lastURIpart">
                                                                <xsl:with-param name="uri" select="@href"/>
                                                        </xsl:call-template>
                                                </xsl:with-param>
                                        </xsl:apply-templates>
                                </xsl:otherwise>
                        </xsl:choose>
                </xsl:otherwise>
        </xsl:choose>
</xsl:template>

ijdickinson commented 9 years ago

Thanks for your contribution - that's exactly how open source software works! I've merged your changes into the codebase.

For future reference, please consider submitting further patches as Github pull requests. That makes it easier to track and apply contributed changes, and ensures that your contribution is more visible to the community. I'd also recommend that you follow local source conventions in any contribution (for example, that .xsl file uses a 2-space indenting convention), and ideally add a test case. A test case will (a) demonstrate that your fix works, and (b) ensure that any future changes don't accidentally re-introduce the problem.

rwalkerands commented 9 years ago

Yeah, happy to construct pull requests, but in this case I didn't think my "contribution" was at the stage of being ready for me to claim it "works". In fact, I'm not sure at all that this patch is right, other than that I've seen it produce the "correct" results in a few cases. I was just hoping someone would in the first instance have a look to see if it was on the right track.

For your comment that the file "uses a 2-space indenting convention", really? The code I copied into the comment had tabs; either my copy/paste or GitHub seems to have converted them to spaces. Oops. (And, indeed, that wouldn't have happened if I'd submitted a pull request.) But the original file seems to use tabs pretty consistently.

epimorphics / elda

Off-the-shelf XSLT doesn't always show labels for nested properties #143