cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vadim Gritsenko <va...@reverycodes.com>
Subject Re: svn commit: r170991 - /cocoon/blocks/core/forms/trunk/java/org/apache/cocoon/forms/transformation/EffectPipe.java
Date Fri, 20 May 2005 16:00:40 GMT
tim@apache.org 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

Mime
View raw message