https://issues.apache.org/bugzilla/show_bug.cgi?id=51052

--- Comment #4 from Andreas L. Delmelle <adelme...@apache.org> 2011-04-14 
14:45:35 EDT ---
(In reply to comment #3)

> However, o.a.f.fo.flow.Marker inherits from FObjMixed, and so when
> Marker.characters() is called, it blindly creates an FOText object without
> checking the validity of the #PCDATA. 

Yep, and at that point you cannot judge whether this will be appropriate, or
not, in the retrieval context. It could be correct for some pages, but wrong on
others, depending on where the retrieve-marker appears.

> One solution would be to store the character array (or as a String) and
> postpone the creation of the FOText node, until the Marker is retrieved by the
> retrieve marker, and thus validation can be done. 

That might be an option. Store the char array, and later on, trigger
characters() on the retrieve-marker, if appropriate.
However, I believe it is not strictly necessary. Also, the current approach of
constructing the FOText early and using clones later, has the convenient
side-effect that the text nodes are stored in sequence with the rest of the
marker's child nodes.

Ultimately, FOText _is_ already basically just a CharBuffer with some extra
information.
The only real gain would be that the FOText property references can be avoided,
so it might still be worthwhile to investigate, but more as a performance
enhancement.

As I see it, what we definitely lack:
- a good/decent way to detect if a FO can have text children; I do not
particularly like 'instanceof FObjMixed', since that does not cover possible
extension classes that subclass FObj directly
- (more general) proper validation of the marker's immediate children against
the parent of the retrieve-marker

The first would solve this particular bug, as it was first reported. The NPE
can be avoided simply by ignoring the FOText (see further below).
The second would be more comprehensive, to avoid similar issues in the future.

> ... It is possible that, just prior to this, we validate the children of the
> marker and either invoke the creation of an FOText object or trigger
> an event to display the error.

Yes, and strictly speaking, this only needs to happen for the first level, as
the other levels are already taken care of during normal FO tree building. 
That is, if you have:

<marker>
  <inline><block>Some text</block></inline>
</marker>

The only validation that is not done, is the inline against the parent of the
retrieve-marker. The block will have been validated for the parent inline,
though, so there is no need to repeat that step for every retrieval.

> I thought this would be a quick fix, but now I'm not so sure.

If someone really needs a quick fix:

* In AbstractRetrieveMarker cloneSingleNode(), before all else, first check

        if (newParent == this && child instanceof FOText
                && !(parent instanceof FObjMixed)) {
            return;
        }

* and add a similar condition --if (parent instanceof FObjMixed) -- 
   around the call to handleWhiteSpaceFor() in cloneFromMarker().

Tested and confirmed that the attachment just renders as a blank page, without
any complaints whatsoever. No more NPE, but this does not address the key
issues mentioned above... :-/

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to