[EMAIL PROTECTED] wrote:
Optimize EffectPipe buffer handling and add locator support to buffers.

I think there are couple of issues here...


Modified: cocoon/blocks/core/forms/trunk/java/org/apache/cocoon/forms/transformation/EffectPipe.java
@@ -174,83 +175,88 @@
*/
protected class BufferHandler extends NullHandler {
public Handler startDocument() throws SAXException {
- if (buffer != null) buffer.startDocument();
+ buffer.startDocument();
return this;
}
+ public void setDocumentLocator(Locator paramLocator) {
+ locator = new LocatorImpl(paramLocator);
+ buffer.setDocumentLocator(paramLocator);
+ }


SaxBuffer does not save locator event. Older version of EffectPipe had local extension of SaxBuffer to store and fire locator events.


@@ -475,6 +484,8 @@
}
this.buffers.addFirst(this.buffer);
}
+ locators.addFirst(locator);
+ locator = new LocatorImpl(locator);
this.buffer = new SaxBuffer();
}
@@ -485,6 +496,7 @@
} else {
this.buffer = null;
}
+ locator = (Locator)locators.removeFirst();
return buffer;
}

This will not preserve locator properly. Check usages on endBuffer: endBuffer only used to obtain the buffer. And when it is later used (like in repeater handler), buffer will reset locator of the EffectPipe, and nothing will restore it.


I think it makes sense to add saveLocator / restoreLocator to handle this situation explicitely.

WDYT?

Vadim

Reply via email to