Fop
  1. Fop
  2. FOP-1550

fo:instream-foreign-object in fo:marker does not work

    Details

    • Type: Bug Bug
    • Status: Closed
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: None
    • Component/s: fo/unqualified
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All
    • External issue ID:
      45306

      Description

      See the disabled testcase added here:
      http://svn.apache.org/viewvc?rev=672617&view=rev

      In current FOP Trunk, the test throws a NPE, due to AbstractRetrieveMarker not correctly cloning the foreign object. cloneSingleNode() apparently only deals with FObj and FOText, but XMLObj descends directly from FONode, so does not survive...

      Changing XMLObj to extend FObj, instead of FONode, partly fixes the issue. It clones the root of the document, but does not yet descend recursively.

        Activity

        Hide
        Luca Furini added a comment -

        I'm wondering whether we really need to clone the whole foreign subtree, as we are quite sure that it's not FO and it's self-contained ....

        I'm attaching a patch that clones the foreign tree root without deleting the child pointers; I have performed a couple of tests and everything seems fine.
        (actually, now I think of it, the patch is slightly sub-optimal, as we could just clone the instream-foreign-object node and share the whole foreign subtree)

        If there are no remaining erroneous situations or objections I can apply the patch to the trunk.

        Show
        Luca Furini added a comment - I'm wondering whether we really need to clone the whole foreign subtree, as we are quite sure that it's not FO and it's self-contained .... I'm attaching a patch that clones the foreign tree root without deleting the child pointers; I have performed a couple of tests and everything seems fine. (actually, now I think of it, the patch is slightly sub-optimal, as we could just clone the instream-foreign-object node and share the whole foreign subtree) If there are no remaining erroneous situations or objections I can apply the patch to the trunk.
        Hide
        Luca Furini added a comment -

        Attachment ifoInsideMarker.080722.diff has been added with description: "cheap" cloning of the foreign subtree

        Show
        Luca Furini added a comment - Attachment ifoInsideMarker.080722.diff has been added with description: "cheap" cloning of the foreign subtree
        Hide
        Andreas L. Delmelle added a comment -

        (In reply to comment #1)

        (Apologies for being so late with the reply)

        > I'm wondering whether we really need to clone the whole foreign subtree, as we
        > are quite sure that it's not FO and it's self-contained ....

        Yep, now that you mention it.

        Seems like it could also be solved by overriding InstreamForeignObject.clone(FONode, boolean), to always pass 'false' for the boolean parameter (= removeChildren)...?

        I'll look into it again later, and see what would work best.

        Thanks for the feedback.

        Show
        Andreas L. Delmelle added a comment - (In reply to comment #1) (Apologies for being so late with the reply) > I'm wondering whether we really need to clone the whole foreign subtree, as we > are quite sure that it's not FO and it's self-contained .... Yep, now that you mention it. Seems like it could also be solved by overriding InstreamForeignObject.clone(FONode, boolean), to always pass 'false' for the boolean parameter (= removeChildren)...? I'll look into it again later, and see what would work best. Thanks for the feedback.
        Hide
        Jeremias Maerki added a comment -

        I've recently fixed that locally without checking for an open Bugzilla entry. Now, I've stumbled over it. I guess I found yet another possible solution. I hope mine is acceptable, too.
        http://svn.apache.org/viewvc?rev=730764&view=rev

        Show
        Jeremias Maerki added a comment - I've recently fixed that locally without checking for an open Bugzilla entry. Now, I've stumbled over it. I guess I found yet another possible solution. I hope mine is acceptable, too. http://svn.apache.org/viewvc?rev=730764&view=rev
        Hide
        Glenn Adams added a comment -

        batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

        Show
        Glenn Adams added a comment - batch transition pre-FOP1.0 resolved+fixed bugs to closed+fixed

          People

          • Assignee:
            fop-dev
            Reporter:
            Andreas L. Delmelle
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development