314985 - Named template stops recursing, output incomplete

Status: NEW

(Core :: XSLT, defect)

Description

[Manfred Staudinger]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7

When file A64x80.xml is displayed using xsl stylesheet view-regest.xsl only 48 of the 64 lines are displayed. In the stylesheet the named template insert-list is called recursively to build nested <ul> constructs depending on the input (number of bbb triplets at the start of each input line). The expected output from the recursion:
<ul><li>line1<ul><li>line2</li>...<li>line64</li></ul></li></ul> 

Reproducible: Always

Steps to Reproduce:
1.If you take file A64x80.xml (64 lines each line 80 bytes long) then you get 48       lines displayed.
2.If you take file A32x160.xml (32 lines each line 160 bytes long) then you get 24 lines displayed.

Actual Results:  
Display files A64x80.xml and A32x160.xml with Firefox; both have a stylesheet view-regest.xsl associated, which is expected to be in the same folder.
Actually only 48 and 24 lines are displayed.

Expected Results:  
Expected to display 64 and 32 lines respectively.

It seems to me that parameter $list2, which is accumulating the lines for the inner list gets too big (still below 5k) and the recursion stops.

Comment 1

[Manfred Staudinger]

Attachment: stylesheet

Comment 2

[Manfred Staudinger]

Attachment: file A64x80.xml to be displayed

Comment 3

[Manfred Staudinger]

Attachment: file A32x160.xml to be displayed

Comment 4

[Manfred Staudinger]

Attachment: copy-paste of the display for A64x80.xml

Comment 5

[Axel Hecht [:Pike]]

Hrm. This is a parser tricky thingie.

The textnode in the source xml get's parsed into several text nodes, which is
likely the culprit here.

In the case of A32x160.xml at least, there are two consecutive text nodes.

Comment 6

[Manfred Staudinger]

>In the case of A32x160.xml at least, there are two consecutive text nodes.
I don't understand this, in both cases the ul element contains one text node!?

The sum of the length of the input "lines" accumulated in $list2 seems to be the trigger. When it reaches a certain limit, the recursion stops. This assumes, the recursion to stop while accumulating input lines ($list2>0), not while generating the li elements ($list2=0). Sounds this reasonable to you? 

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So this should probably be a dup on some bug that the node-tree we expose to XPath can contain text() nodes that are siblings of each other.

What we need to do is to let the walkers 'merge' adjecent textnodes. However we need to do this in a performant way.

In most cases things work just fine. It's really only when using text() or node() filters that things can go bad, so we should only in those cases do the merging.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Manfred: As a workaround until the real bug is fixed. Change

<xsl:with-param name="list" select="text()"/>

to

<xsl:with-param name="list" select="."/>

Comment 9

[Manfred Staudinger]

workaround <xsl:with-param name="list" select="."/> works
fine. I should have tried this myself ... Thanks 

Comment 10

[Manfred Staudinger]

Could you please look into this, it might be the very same bug:
1. Invoke http://free.pages.at/staudinger/testff/A1598-06-26-01484.xml
2. Between the last 2 paragraphs displayed, there are about 80+ bytes 
   of text missing. In addition these two paragraphs should be one
   instead.
3. The stylesheet at 
   http://free.pages.at/staudinger/testff/view-regest-a.xsl
   has this template:
   <xsl:template match="xhtml:zitat/text()">
      <xsl:call-template name="insertParagraphs">
         <xsl:with-param name="quote" select="."/>
      </xsl:call-template>
   </xsl:template>
4. The stylesheet output is correct with SAXON 6.5.4

Comment 11

[Manfred Staudinger]

You may have allready noticed: the breaking point ist after
4000 Bytes and data up to a full input line may be lost.
Also I dont have a workaround here!

Comment 12

[Manfred Staudinger]

Sorry, I forgot to mention: this is on Fx 1.5!

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The workaround is same as before. Instead of

match="xhtml:zitat/text()"

use

match="xhtml:zitat"

Why do you insist on navigating to the textnode all the time, stick to the element ;)

Comment 14

[Manfred Staudinger]

In the middle of the text() there may be one or more element nodes,
e.g. ul, which need to be processed separately. In order to make
each of those text() nodes to become a child node of e.g. a p element,
I think I cannot use match="xhtml:zitat" but have to stick with
match="xhtml:zitat/text()".

The input xml may look like (snipe):
<zitat>
text
<ul>text</ul>
text
<ol>text</ol>
text
</zitat>

for this the output should be:
<blockquote>
<p>text</p>
<ul><li>text</li>...<li>text</li></ul>
<p>text</p>
<ol><li>text</li>...<li>text</li></ol>
<pre>text</pre>
</blockquote>
 
This is not shown in the stylesheet, which is a trimmed down version,
as per bugzilla requirement. The real stylesheet, which currently
processes about 2700 different xml documents, is slightly more
complex. But I'm open for any suggestion for a workaround.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ok, that one is trickier, but with a little XPath magic it can be done.

First, change

match="xhtml:zitat/text()"

to

match="xhtml:zitat/text()[not(preceding-sibling::node()[1][self::text()])]"



Then change

      <xsl:call-template name="insertParagraphs">
         <xsl:with-param name="quote" select="."/>
      </xsl:call-template>

to

      <xsl:variable name="q">
        <xsl:call-template name="getTextValue"/>
      </xsl:variable>
      <xsl:call-template name="insertParagraphs">
         <xsl:with-param name="quote" select="string($q)"/>
      </xsl:call-template>

and add the template

<xsl:template name="getTextValue">
  <xsl:value-of select=".">
  <xsl:for-each select="following-sibling::node()[1][self::text()]">
    <xsl:call-template name="getTextValue"/>
  </xsl:for-each>
</xsl:template>


This *should* work, but I havn't tested it so there might be some minor errors.

I realize that this is a lot of hassle. I have a patch that should fix this bug in the common case, but you'll have to use the workaround if you want to work on already released versions of firefox/mozilla.

Comment 16

[Manfred Staudinger]

Works great! I only had to add
<xsl:template match="xhtml:zitat/text()[position()>1]"/>

>I realize that this is a lot of hassle.
No, its a pleasure to work with you. Thanks

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> Works great! I only had to add
> <xsl:template match="xhtml:zitat/text()[position()>1]"/>

Uh? That would make all but the first textnodes do nothing at all? That doesn't seem like what you want?

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch v1

So this make us behave correctly for PI transformations

Comment 19

[Manfred Staudinger]

>> Works great! I only had to add
>> <xsl:template match="xhtml:zitat/text()[position()>1]"/>

>Uh? That would make all but the first textnodes do nothing at all? 

Exactly. Position is define by context, so in fact instead of
match="xhtml:zitat/text()[not(preceding-sibling::node()[1][self::text()])]"
it is sufficient to write
match="xhtml:zitat/text()[1]"

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

but for something like
<zitat>
  textfoo
  <ol/>
  textbar
</zitat>

you'll never output the 'textbar' part.

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ah, i think i know what's happening. Your template appears before mine in the document which means that mine gets priority if both match. The end result is that your template will just hide the remaining textnodes. You could do that by simply doing:

<xsl:template priority="-1" match="xhtml:zitat/text()"/>

You still need to use

match="xhtml:zitat/text()[not(preceding-sibling::node()[1][self::text()])]"

though, just

match="xhtml:zitat/text()[1]"

is not good enough.

Comment 22

[Manfred Staudinger]

>You still need to use
>match="xhtml:zitat/text()[not(preceding-sibling::node()[1][self::text()])]"
>though, just
>match="xhtml:zitat/text()[1]"
>is not good enough.
Correct. I was misled by my position()>1 and the fact that saxon did
not warn about ambiguous matches (of course, it has no "secondary" text
nodes)

><xsl:template priority="-1" match="xhtml:zitat/text()"/>
That's a perfect solution now! Thanks!

Comment 23

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 207668 [details] [diff] [review]
Patch v1

a=shaver for 1.8 branch.

Comment 24

[Peter Van der Beken [:peterv]]

Sicking: I think this can be marked fixed?

Comment 25

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It's only fixed for PI transforms. If you use the JS interface adjecent textnodes are still treated as separate nodes. What we ideally should do is let the treewalker behave as if the nodes have been merged.

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.