Named template stops recursing, output incomplete

ASSIGNED
Assigned to

Status

()

Core
XSLT
ASSIGNED
12 years ago
8 years ago

People

(Reporter: Manfred Staudinger, Assigned: sicking)

Tracking

({fixed1.8.1})

Trunk
x86
Windows 2000
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 1

12 years ago
Created attachment 201780 [details]
stylesheet
(Reporter)

Comment 2

12 years ago
Created attachment 201781 [details]
file A64x80.xml to be displayed
(Reporter)

Comment 3

12 years ago
Created attachment 201782 [details]
file A32x160.xml to be displayed
(Reporter)

Comment 4

12 years ago
Created attachment 201783 [details]
copy-paste of the display for A64x80.xml
(Reporter)

Updated

12 years ago
Attachment #201781 - Attachment description: file to be displaed → file A64x80.xml to be displayed
(Reporter)

Updated

12 years ago
Attachment #201782 - Attachment description: file to be displaed → file A32x160.xml to be displayed

Comment 5

12 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 6

12 years ago
>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? 
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.
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="."/>
(Reporter)

Comment 9

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

Comment 10

12 years ago
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
(Reporter)

Comment 11

12 years ago
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!
(Reporter)

Comment 12

12 years ago
Sorry, I forgot to mention: this is on Fx 1.5!
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 ;)
(Reporter)

Comment 14

12 years ago
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.
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.
(Reporter)

Comment 16

12 years ago
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
> 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?
Created attachment 207668 [details] [diff] [review]
Patch v1

So this make us behave correctly for PI transformations
Assignee: peterv → bugmail
Status: NEW → ASSIGNED
Attachment #207668 - Flags: superreview?(peterv)
Attachment #207668 - Flags: review?(peterv)
(Reporter)

Comment 19

12 years ago
>> 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]"
but for something like
<zitat>
  textfoo
  <ol/>
  textbar
</zitat>

you'll never output the 'textbar' part.
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.
(Reporter)

Comment 22

12 years ago
>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!


Attachment #207668 - Flags: superreview?(peterv)
Attachment #207668 - Flags: superreview+
Attachment #207668 - Flags: review?(peterv)
Attachment #207668 - Flags: review+
Attachment #207668 - Flags: approval-branch-1.8.1?
Comment on attachment 207668 [details] [diff] [review]
Patch v1

a=shaver for 1.8 branch.
Attachment #207668 - Flags: approval-branch-1.8.1? → approval-branch-1.8.1+
Keywords: fixed1.8.1
Sicking: I think this can be marked fixed?
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.
QA Contact: keith → xslt
You need to log in before you can comment on or make changes to this bug.