Bugzilla 44634: Added partial implementation for show-destination.

Only works for PDF output, when the PDF is opened in a standalone viewer, and for links that point to destinations in another PDF document.


git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@651575 13f79535-47bb-0310-9956-ffa450edef68

10 files changed, 205 insertions, 190 deletions

diff --git a/src/java/org/apache/fop/area/AreaTreeParser.java b/src/java/org/apache/fop/area/AreaTreeParser.java
index 19edd3d5e..d48221c0a 100644
--- a/src/java/org/apache/fop/area/AreaTreeParser.java
+++ b/src/java/org/apache/fop/area/AreaTreeParser.java
@@ -1018,7 +1018,7 @@ public class AreaTreeParser {
             Trait.IS_REFERENCE_AREA, Trait.IS_VIEWPORT_AREA};
 
         private void setTraits(Attributes attributes, Area area, Object[] traitSubset) {
-            for (int i = 0, c = traitSubset.length; i < c; i++) {
+            for (int i = traitSubset.length; --i >= 0;) {
                 Object trait = traitSubset[i];
                 String traitName = Trait.getTraitName(trait);
                 String value = attributes.getValue(traitName);
@@ -1044,6 +1044,8 @@ public class AreaTreeParser {
                         }
                     } else if (cl == InternalLink.class) {
                         area.addTrait(trait, new InternalLink(value));
+                    } else if (cl == Trait.ExternalLink.class) {
+                        area.addTrait(trait, Trait.ExternalLink.makeFromTraitValue(value));
                     } else if (cl == Background.class) {
                         Background bkg = new Background();
                         try {
diff --git a/src/java/org/apache/fop/area/Trait.java b/src/java/org/apache/fop/area/Trait.java
index 98bed098c..9599d8833 100644
--- a/src/java/org/apache/fop/area/Trait.java
+++ b/src/java/org/apache/fop/area/Trait.java
@@ -194,7 +194,7 @@ public class Trait implements Serializable {
     public static final Integer OVERLINE_COLOR = new Integer(35);
     /** Trait for color of linethrough decorations when rendering inline parent. */
     public static final Integer LINETHROUGH_COLOR = new Integer(36);
-    
+
     /** Maximum value used by trait keys */
     public static final int MAX_TRAIT_KEY = 36;
     
@@ -226,7 +226,7 @@ public class Trait implements Serializable {
         // Create a hashmap mapping trait code to name for external representation
         //put(ID_LINK, new TraitInfo("id-link", String.class));
         put(INTERNAL_LINK, new TraitInfo("internal-link", InternalLink.class));
-        put(EXTERNAL_LINK, new TraitInfo("external-link", String.class));
+        put(EXTERNAL_LINK, new TraitInfo("external-link", ExternalLink.class));
         put(FONT,         new TraitInfo("font", FontTriplet.class));
         put(FONT_SIZE,    new TraitInfo("font-size", Integer.class));
         put(COLOR, new TraitInfo("color", Color.class));
@@ -277,7 +277,7 @@ public class Trait implements Serializable {
                 new TraitInfo("is-reference-area", Boolean.class));
         put(IS_VIEWPORT_AREA,
                 new TraitInfo("is-viewport-area", Boolean.class));
-        
+
     }
 
     /**
@@ -291,25 +291,6 @@ public class Trait implements Serializable {
     }
 
     /**
-     * Get the trait code for a trait name.
-     *
-     * @param sTraitName the name of the trait to find
-     * @return the trait code object
-     */
-    /*
-    public static Object getTraitCode(String sTraitName) {
-        Iterator iter = TRAIT_INFO.entrySet().iterator();
-        while (iter.hasNext()) {
-            Map.Entry entry = (Map.Entry) iter.next();
-            TraitInfo ti = (TraitInfo) entry.getValue();
-            if (ti != null && ti.getName().equals(sTraitName)) {
-                return entry.getKey();
-            }
-        }
-        return null;
-    }*/
-
-    /**
      * Get the data storage class for the trait.
      *
      * @param traitCode the trait code to lookup
@@ -320,99 +301,6 @@ public class Trait implements Serializable {
     }
 
     /**
-     * The type of trait for an area.
-     */
-    private Object propType;
-
-    /**
-     * The data value of the trait.
-     */
-    private Object data;
-
-    /**
-     * Create a new empty trait.
-     */
-    public Trait() {
-        this.propType = null;
-        this.data = null;
-    }
-
-    /**
-     * Create a trait with the value and type.
-     *
-     * @param propType the type of trait
-     * @param data the data value
-     */
-    public Trait(Object propType, Object data) {
-        this.propType = propType;
-        this.data = data;
-    }
-
-    /**
-     * Returns the trait data value.
-     * @return the trait data value
-     */
-    public Object getData() {
-        return this.data;
-    }
-
-    /**
-     * Returns the property type.
-     * @return the property type
-     */
-    public Object getPropType() {
-        return this.propType;
-    }
-
-    /**
-     * Return the string for debugging.
-     * {@inheritDoc}
-     */
-    public String toString() {
-        return data.toString();
-    }
-
-    /**
-     * Make a trait value.
-     *
-     * @param oCode trait code
-     * @param sTraitValue trait value as String
-     * @return the trait value as object
-     */
-    /*
-    public static Object makeTraitValue(Object oCode, String sTraitValue) {
-        // Get the code from the name
-        // See what type of object it is
-        // Convert string value to an object of that type
-        Class tclass = getTraitClass(oCode);
-        if (tclass == null) {
-            return null;
-        }
-        if (tclass.equals(String.class)) {
-            return sTraitValue;
-        }
-        if (tclass.equals(Integer.class)) {
-            return new Integer(sTraitValue);
-        }
-        // See if the class has a constructor from string or can read from a string
-        try {
-            Object o = tclass.newInstance();
-            //return o.fromString(sTraitValue);
-        } catch (IllegalAccessException e1) {
-            log.error("Can't create instance of "
-                               + tclass.getName());
-            return null;
-        } catch (InstantiationException e2) {
-            log.error("Can't create instance of "
-                               + tclass.getName());
-            return null;
-        }
-
-
-        return null;
-    }*/
-
-    /**
      * Class for internal link traits.
      * Stores PageViewport key and producer ID
      */
@@ -548,6 +436,80 @@ public class Trait implements Serializable {
     }
 
     /**
+     * External Link trait structure
+     */
+    public static class ExternalLink implements Serializable {
+
+        private String destination;
+        private boolean newWindow;
+
+        /**
+         * Constructs an ExternalLink object with the given destination
+         *
+         * @param destination   target of the link
+         * @param newWindow     true if the target should be opened in a new window
+         */
+        public ExternalLink(String destination, boolean newWindow) {
+            this.destination = destination;
+            this.newWindow = newWindow;
+        }
+
+        /**
+         * Create an <code>ExternalLink</code> from a trait value/attribute value in the
+         * area tree
+         * @param traitValue    the value to use (should match the result of {@link #toString()}
+         * @return an <code>ExternalLink</code> instance corresponding to the given value
+         */
+        protected static ExternalLink makeFromTraitValue(String traitValue) {
+            if (traitValue.indexOf(ExternalLink.class.getName()) == -1
+                    || traitValue.indexOf("dest=") == -1) {
+                throw new IllegalArgumentException(
+                        "Malformed trait value for Trait.ExternalLink: " + traitValue);
+            }
+            int startIndex = traitValue.indexOf("dest=") + 5;
+            int endIndex = traitValue.indexOf(',', startIndex);
+            if (endIndex == -1) {
+                endIndex = traitValue.indexOf(']');
+            }
+            String dest = traitValue.substring(startIndex, endIndex);
+            startIndex = traitValue.indexOf("newWindow=", endIndex) + 10;
+            endIndex = traitValue.indexOf(']', startIndex);
+            boolean newWindow = Boolean.parseBoolean(
+                    traitValue.substring(startIndex, endIndex));
+            return new ExternalLink(dest, newWindow);
+        }
+
+        /**
+         * Get the target/destination of the link
+         * @return  the destination of the link
+         */
+        public String getDestination() {
+            return this.destination;
+        }
+
+        /**
+         * Check if the target has to be displayed in a new window
+         * @return  <code>true</code> if the target has to be displayed in a new window
+         */
+        public boolean newWindow() {
+            return this.newWindow;
+        }
+
+        /**
+         * Return a String representation of the object.
+         * @return  a <code>String</code> of the form
+         *          "org.apache.fop.area.Trait.ExternalLink[dest=someURL,newWindow=false]"
+         */
+        public String toString() {
+            StringBuffer sb = new StringBuffer(64);
+            sb.append(super.toString());
+            sb.append("[dest=").append(this.destination);
+            sb.append(",newWindow=").append(newWindow).append("]");
+            return sb.toString();
+        }
+    }
+
+    /**
      * Background trait structure.
      * Used for storing back trait information which are related.
      */
diff --git a/src/java/org/apache/fop/fo/Constants.java b/src/java/org/apache/fop/fo/Constants.java
index edfa68c1a..25c2fe371 100644
--- a/src/java/org/apache/fop/fo/Constants.java
+++ b/src/java/org/apache/fop/fo/Constants.java
@@ -1092,7 +1092,11 @@ public interface Constants {
     /** Enumeration constant -- for instream-foreign-object and external-graphic, XSL 1.1 */
     int EN_SCALE_DOWN_TO_FIT = 187; 
     /** Enumeration constant -- for instream-foreign-object and external-graphic, XSL 1.1 */
-    int EN_SCALE_UP_TO_FIT = 188; 
+    int EN_SCALE_UP_TO_FIT = 188;
+    /** Enumeration constant -- for fo:basic-link show-destination */
+    int EN_REPLACE = 189;
+    /** Enumeration constant -- for fo:basic-link show-destination */
+    int EN_NEW = 190;
     /** Number of enumeration constants defined */
-    int ENUM_COUNT = 188;
+    int ENUM_COUNT = 190;
 }
diff --git a/src/java/org/apache/fop/fo/FOPropertyMapping.java b/src/java/org/apache/fop/fo/FOPropertyMapping.java
index db19d6515..5d4185d68 100644
--- a/src/java/org/apache/fop/fo/FOPropertyMapping.java
+++ b/src/java/org/apache/fop/fo/FOPropertyMapping.java
@@ -258,7 +258,7 @@ public final class FOPropertyMapping implements Constants {
 
     /**
      * Return a (possibly cached) enum property based in the enum value.
-     * @param enum A enum value from Constants.java.
+     * @param enumValue A enum value from Constants.java.
      * @param text the text value by which this enum property is known
      * @return An EnumProperty instance.
      */
@@ -371,9 +371,8 @@ public final class FOPropertyMapping implements Constants {
                 || ((id & Constants.PROPERTY_MASK) == 0)) {
             return (String) s_htPropIds.get(new Integer(id));
         } else {
-            return (String) s_htPropIds.get(new Integer(
-                    id & Constants.PROPERTY_MASK)) + "." + s_htPropIds.get(
-                            new Integer(id & Constants.COMPOUND_MASK));
+            return s_htPropIds.get(new Integer(id & Constants.PROPERTY_MASK))
+                    + "." + s_htPropIds.get(new Integer(id & Constants.COMPOUND_MASK));
         }
     }
 
@@ -2019,8 +2018,10 @@ public final class FOPropertyMapping implements Constants {
         addPropertyMaker("internal-destination", m);
 
         // show-destination
-        m  = new ToBeImplementedProperty.Maker(PR_SHOW_DESTINATION);
+        m  = new EnumProperty.Maker(PR_SHOW_DESTINATION);
         m.setInherited(false);
+        m.addEnum("new", getEnumProperty(EN_NEW, "NEW"));
+        m.addEnum("replace", getEnumProperty(EN_REPLACE, "REPLACE"));
         m.setDefault("replace");
         addPropertyMaker("show-destination", m);
 
diff --git a/src/java/org/apache/fop/fo/flow/BasicLink.java b/src/java/org/apache/fop/fo/flow/BasicLink.java
index b3ef48012..82d0134bd 100644
--- a/src/java/org/apache/fop/fo/flow/BasicLink.java
+++ b/src/java/org/apache/fop/fo/flow/BasicLink.java
@@ -27,19 +27,21 @@ import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
 
 /**
- * Class modelling the fo:basic-link object.
+ * Class modelling the <a href="http://www.w3.org/TR/xsl/#fo_basic-link">
+ * <code>fo:basic-link</code></a> object.
  *
  * This class contains the logic to determine the link represented by this FO,
  * and whether that link is external (uses a URI) or internal (an id 
  * reference).
  */
 public class BasicLink extends Inline {
+
     // The value of properties relevant for fo:basic-link.
     // private ToBeImplementedProperty destinationPlacementOffset;
     private String externalDestination;
     // private ToBeImplementedProperty indicateDestination;
     private String internalDestination;
-    // private ToBeImplementedProperty showDestination;
+    private int showDestination;
     // private ToBeImplementedProperty targetProcessingContext;
     // private ToBeImplementedProperty targetPresentationContext;
     // private ToBeImplementedProperty targetStylesheet;
@@ -51,22 +53,23 @@ public class BasicLink extends Inline {
     private boolean blockOrInlineItemFound = false;
 
     /**
-     * @param parent FONode that is the parent of this object
+     * Construct a BasicLink instance with the given {@link FONode}
+     * as its parent.
+     * 
+     * @param parent {@link FONode} that is the parent of this object
      */
     public BasicLink(FONode parent) {
         super(parent);
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public void bind(PropertyList pList) throws FOPException {
         super.bind(pList);
         // destinationPlacementOffset = pList.get(PR_DESTINATION_PLACEMENT_OFFSET);
         externalDestination = pList.get(PR_EXTERNAL_DESTINATION).getString();
         // indicateDestination = pList.get(PR_INDICATE_DESTINATION);
         internalDestination = pList.get(PR_INTERNAL_DESTINATION).getString();
-        // showDestination = pList.get(PR_SHOW_DESTINATION);
+        showDestination = pList.get(PR_SHOW_DESTINATION).getEnum();
         // targetProcessingContext = pList.get(PR_TARGET_PROCESSING_CONTEXT);
         // targetPresentationContext = pList.get(PR_TARGET_PRESENTATION_CONTEXT);
         // targetStylesheet = pList.get(PR_TARGET_STYLESHEET);
@@ -80,26 +83,19 @@ public class BasicLink extends Inline {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startLink(this);
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     protected void endOfNode() throws FOPException {
         super.endOfNode();
         getFOEventHandler().endLink();
     }
 
-    /**
-     * {@inheritDoc} String, String)
-     * XSL Content Model: marker* (#PCDATA|%inline;|%block;)*
-     */
+    /** {@inheritDoc} */
     protected void validateChildNode(Locator loc, String nsURI, String localName) 
                 throws ValidationException {
         if (FO_URI.equals(nsURI)) {
@@ -116,39 +112,61 @@ public class BasicLink extends Inline {
     }
 
     /**
-     * @return the "internal-destination" property.
+     * Get the value of the <code>internal-destination</code> property.
+     *
+     * @return the "internal-destination" property
      */
     public String getInternalDestination() {
         return internalDestination;
     }
 
     /**
-     * @return the "external-destination" property.
+     * Get the value of the <code>external-destination</code> property.
+     *
+     * @return the "external-destination" property
      */
     public String getExternalDestination() {
         return externalDestination;
     }
 
     /**
-     * @return whether or not this basic link has an internal destination or not
+     * Convenience method to check if this instance has an internal destination.
+     *
+     * @return <code>true</code> if this basic link has an internal destination;
+     *          <code>false</code> otherwise
      */
     public boolean hasInternalDestination() {
         return internalDestination != null && internalDestination.length() > 0;
     }
 
     /**
-     * @return whether or not this basic link has an external destination or not
+     * Convenience method to check if this instance has an external destination
+     *
+     * @return <code>true</code> if this basic link has an external destination;
+     *          <code>false</code> otherwise
      */
     public boolean hasExternalDestination() {
         return externalDestination != null && externalDestination.length() > 0;
     }
 
+    /**
+     * Get the value of the <code>show-destination</code> property.
+     *
+     * @return the "show-destination" property
+     */
+    public int getShowDestination() {
+        return this.showDestination;
+    }
+
     /** {@inheritDoc} */
     public String getLocalName() {
         return "basic-link";
     }
     
-    /** {@inheritDoc} */
+    /**
+     * {@inheritDoc}
+     * @return {@link org.apache.fop.fo.Constants#FO_BASIC_LINK}
+     */
     public int getNameId() {
         return FO_BASIC_LINK;
     }
diff --git a/src/java/org/apache/fop/layoutmgr/inline/BasicLinkLayoutManager.java b/src/java/org/apache/fop/layoutmgr/inline/BasicLinkLayoutManager.java
index e7339034c..b8979e187 100644
--- a/src/java/org/apache/fop/layoutmgr/inline/BasicLinkLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/inline/BasicLinkLayoutManager.java
@@ -21,7 +21,7 @@ package org.apache.fop.layoutmgr.inline;
 
 import org.apache.fop.datatypes.URISpecification;
 import org.apache.fop.fo.flow.BasicLink;
-import org.apache.fop.layoutmgr.LayoutManager;
+import org.apache.fop.fo.Constants;
 import org.apache.fop.layoutmgr.PageSequenceLayoutManager;
 import org.apache.fop.area.inline.InlineArea;
 import org.apache.fop.area.Trait;
@@ -31,7 +31,6 @@ import org.apache.fop.area.LinkResolver;
  * LayoutManager for the fo:basic-link formatting object
  */
 public class BasicLinkLayoutManager extends InlineLayoutManager {
-    private BasicLink fobj;
 
     /**
      * Create an fo:basic-link layout manager.
@@ -40,23 +39,22 @@ public class BasicLinkLayoutManager extends InlineLayoutManager {
      */
     public BasicLinkLayoutManager(BasicLink node) {
         super(node);
-        fobj = node;
     }
 
     /** {@inheritDoc} */
     protected InlineArea createArea(boolean bInlineParent) {
         InlineArea area = super.createArea(bInlineParent);
-        setupBasicLinkArea(parentLM, area);
+        setupBasicLinkArea(area);
         return area;
     }
 
     /*
      * Detect internal or external link and add it as an area trait
      *
-     * @param parentLM the parent LayoutManager
      * @param area the basic-link's area
      */
-    private void setupBasicLinkArea(LayoutManager parentLM, InlineArea area) {
+    private void setupBasicLinkArea(InlineArea area) {
+        BasicLink fobj = (BasicLink) this.fobj;
         // internal destinations take precedence:
         if (fobj.hasInternalDestination()) {
             String idref = fobj.getInternalDestination();
@@ -70,8 +68,10 @@ public class BasicLinkLayoutManager extends InlineLayoutManager {
             }
         } else if (fobj.hasExternalDestination()) {
             String url = URISpecification.getURL(fobj.getExternalDestination());
+            boolean newWindow = (fobj.getShowDestination() == Constants.EN_NEW);
             if (url.length() > 0) {
-                area.addTrait(Trait.EXTERNAL_LINK, url);
+                area.addTrait(Trait.EXTERNAL_LINK,
+                        new Trait.ExternalLink(url, newWindow));
             }
         }
     }
diff --git a/src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java b/src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java
index f3bb66022..b59f0466a 100755
--- a/src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java
@@ -68,8 +68,6 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
      */
     private static Log log = LogFactory.getLog(InlineLayoutManager.class);
 
-    private InlineLevel fobj;
-
     private CommonMarginInline inlineProps = null;
     private CommonBorderPaddingBackground borderProps = null;
 
@@ -105,7 +103,6 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
     // The node should be FObjMixed
     public InlineLayoutManager(InlineLevel node) {
         super(node);
-        fobj = node;
     }
     
     private Inline getInlineFO() {
@@ -114,6 +111,8 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
     
     /** {@inheritDoc} */
     public void initialize() {
+        InlineLevel fobj = (InlineLevel) this.fobj;
+
         int padding = 0;
         FontInfo fi = fobj.getFOEventHandler().getFontInfo();
         FontTriplet[] fontkeys = fobj.getCommonFont().getFontState(fi);
@@ -555,7 +554,8 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
         if (returnList instanceof BlockKnuthSequence) {
             return;
         }
-        CommonBorderPaddingBackground borderAndPadding = fobj.getCommonBorderPaddingBackground();
+        CommonBorderPaddingBackground borderAndPadding =
+                ((InlineLevel)fobj).getCommonBorderPaddingBackground();
         if (borderAndPadding != null) {
             int ipStart = borderAndPadding.getBorderStartWidth(false)
                          + borderAndPadding.getPaddingStart(false, this);
@@ -579,7 +579,8 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
         if (returnList instanceof BlockKnuthSequence) {
             return;
         }
-        CommonBorderPaddingBackground borderAndPadding = fobj.getCommonBorderPaddingBackground();
+        CommonBorderPaddingBackground borderAndPadding =
+                ((InlineLevel)fobj).getCommonBorderPaddingBackground();
         if (borderAndPadding != null) {
             int ipEnd = borderAndPadding.getBorderEndWidth(false)
                         + borderAndPadding.getPaddingEnd(false, this);
diff --git a/src/java/org/apache/fop/pdf/PDFFactory.java b/src/java/org/apache/fop/pdf/PDFFactory.java
index f4474331f..02f0c2cdb 100644
--- a/src/java/org/apache/fop/pdf/PDFFactory.java
+++ b/src/java/org/apache/fop/pdf/PDFFactory.java
@@ -724,6 +724,7 @@ public class PDFFactory {
      * @param theColors the list of colors for the gradient
      * @param theBounds the list of bounds associated with the colors
      * @param theCoords the coordinates for the gradient
+     * @param theMatrix the coordinate-transformation matrix
      * @return the PDF pattern that was created
      */
     public PDFPattern makeGradient(PDFResourceContext res, boolean radial,
@@ -737,7 +738,7 @@ public class PDFFactory {
         List theCone;
         PDFPattern myPattern;
         //PDFColorSpace theColorSpace;
-        double interpolation = (double)1.000;
+        double interpolation = 1.000;
         List theFunctions = new ArrayList();
 
         int currentPosition;
@@ -874,7 +875,8 @@ public class PDFFactory {
      */
     public PDFDests makeDests(List destinationList) {
         PDFDests dests;
-        
+
+        //TODO: Check why the below conditional branch is needed. Condition is always true...
         final boolean deep = true;
         //true for a "deep" structure (one node per entry), true for a "flat" structure
         if (deep) {
@@ -961,7 +963,7 @@ public class PDFFactory {
     }
 
     /**
-     * make a link object
+     * Make a {@link PDFLink} object
      *
      * @param rect   the clickable rectangle
      * @param destination  the destination file
@@ -976,7 +978,7 @@ public class PDFFactory {
         PDFLink link = new PDFLink(rect);
 
         if (linkType == PDFLink.EXTERNAL) {
-            link.setAction(getExternalAction(destination));
+            link.setAction(getExternalAction(destination, false));
         } else {
             // linkType is internal
             String goToReference = getGoToReference(destination, yoffset);
@@ -999,9 +1001,11 @@ public class PDFFactory {
      *
      * @param target The external target. This may be a PDF file name
      * (optionally with internal page number or destination) or any type of URI.
+     * @param newWindow boolean indicating whether the target should be
+     *                  displayed in a new window
      * @return the PDFAction thus created or found
      */
-    public PDFAction getExternalAction(String target) {
+    public PDFAction getExternalAction(String target, boolean newWindow) {
         int index;
         String targetLo = target.toLowerCase();
         // HTTP URL?
@@ -1009,17 +1013,17 @@ public class PDFFactory {
             return new PDFUri(target);
         // Bare PDF file name?
         } else if (targetLo.endsWith(".pdf")) {
-            return getGoToPDFAction(target, null, -1);
+            return getGoToPDFAction(target, null, -1, newWindow);
         // PDF file + page?
         } else if ((index = targetLo.indexOf(".pdf#page=")) > 0) {
             String filename = target.substring(0, index + 4);
             int page = Integer.parseInt(target.substring(index + 10));
-            return getGoToPDFAction(filename, null, page);
+            return getGoToPDFAction(filename, null, page, newWindow);
         // PDF file + destination?
         } else if ((index = targetLo.indexOf(".pdf#dest=")) > 0) {
             String filename = target.substring(0, index + 4);
             String dest = target.substring(index + 10);
-            return getGoToPDFAction(filename, dest, -1);
+            return getGoToPDFAction(filename, dest, -1, newWindow);
         // None of the above? Default to URI:
         } else {
             return new PDFUri(target);
@@ -1069,9 +1073,11 @@ public class PDFFactory {
      * @param file the pdf file name
      * @param dest the remote name destination, may be null
      * @param page the remote page number, -1 means not specified
+     * @param newWindow boolean indicating whether the target should be
+     *                  displayed in a new window
      * @return the pdf goto remote object
      */
-    private PDFGoToRemote getGoToPDFAction(String file, String dest, int page) {
+    private PDFGoToRemote getGoToPDFAction(String file, String dest, int page, boolean newWindow) {
         getDocument().getProfile().verifyActionAllowed();
         PDFFileSpec fileSpec = new PDFFileSpec(file);
         PDFFileSpec oldspec = getDocument().findFileSpec(fileSpec);
@@ -1083,11 +1089,11 @@ public class PDFFactory {
         PDFGoToRemote remote;
 
         if (dest == null && page == -1) {
-            remote = new PDFGoToRemote(fileSpec);
+            remote = new PDFGoToRemote(fileSpec, newWindow);
         } else if (dest != null) {
-            remote = new PDFGoToRemote(fileSpec, dest);
+            remote = new PDFGoToRemote(fileSpec, dest, newWindow);
         } else {
-            remote = new PDFGoToRemote(fileSpec, page);
+            remote = new PDFGoToRemote(fileSpec, page, newWindow);
         }
         PDFGoToRemote oldremote = getDocument().findGoToRemote(remote);
         if (oldremote == null) {
@@ -1197,8 +1203,7 @@ public class PDFFactory {
             PDFFontDescriptor pdfdesc = makeFontDescriptor(descriptor);
 
             PDFFont font = null;
-            font = (PDFFont)PDFFont.createFont(fontname, fonttype,
-                                                        basefont, encoding);
+            font = PDFFont.createFont(fontname, fonttype, basefont, encoding);
             getDocument().registerObject(font);
 
             if (fonttype == FontType.TYPE0) {
@@ -1298,6 +1303,7 @@ public class PDFFactory {
     /**
      * Creates a PDFEncoding instance from a CodePointMapping instance.
      * @param encoding the code point mapping (encoding)
+     * @param fontNameHint ...
      * @return the PDF Encoding dictionary (or a String with the predefined encoding)
      */
     public Object createPDFEncoding(SingleByteEncoding encoding, String fontNameHint) {
@@ -1458,6 +1464,7 @@ public class PDFFactory {
                 try {
                     in = new java.net.URL(source.getSystemId()).openStream();
                 } catch (MalformedURLException e) {
+                    //TODO: Why construct a new exception here, when it is not thrown?
                     new FileNotFoundException(
                             "File not found. URL could not be resolved: "
                                     + e.getMessage());
@@ -1514,7 +1521,7 @@ public class PDFFactory {
             log.error(
                     "Failed to embed font [" + desc + "] "
                     + desc.getEmbedFontName(), ioe);
-            return (PDFStream) null;
+            return null;
         }
     }
 
@@ -1563,7 +1570,6 @@ public class PDFFactory {
     /**
      * Create a PDFICCStream
      * @see PDFImageXObject
-     * @see org.apache.fop.image.JpegImage
      * @see org.apache.fop.pdf.PDFDeviceColorSpace     
      * @return the new PDF ICC stream object
      */
diff --git a/src/java/org/apache/fop/pdf/PDFGoToRemote.java b/src/java/org/apache/fop/pdf/PDFGoToRemote.java
index 71cae5ba6..2cd937df3 100644
--- a/src/java/org/apache/fop/pdf/PDFGoToRemote.java
+++ b/src/java/org/apache/fop/pdf/PDFGoToRemote.java
@@ -20,7 +20,7 @@
 package org.apache.fop.pdf;
 
 /**
- * class representing a /GoToR object.
+ * Class representing a /GoToR object.
  */
 public class PDFGoToRemote extends PDFAction {
 
@@ -30,17 +30,21 @@ public class PDFGoToRemote extends PDFAction {
     private PDFFileSpec pdfFileSpec;
     private int pageReference = 0;
     private String destination = null;
+    private boolean newWindow = false;
 
     /**
-     * create an GoToR object.
+     * Create an GoToR object.
      *
      * @param pdfFileSpec the fileSpec associated with the action
+     * @param newWindow boolean indicating whether the target should be
+     *                  displayed in a new window
      */
-    public PDFGoToRemote(PDFFileSpec pdfFileSpec) {
+    public PDFGoToRemote(PDFFileSpec pdfFileSpec, boolean newWindow) {
         /* generic creation of object */
         super();
 
         this.pdfFileSpec = pdfFileSpec;
+        this.newWindow = newWindow;
     }
 
     /**
@@ -48,13 +52,16 @@ public class PDFGoToRemote extends PDFAction {
      *
      * @param pdfFileSpec the fileSpec associated with the action
      * @param page a page reference within the remote document
+     * @param newWindow boolean indicating whether the target should be
+     *                  displayed in a new window
      */
-    public PDFGoToRemote(PDFFileSpec pdfFileSpec, int page) {
+    public PDFGoToRemote(PDFFileSpec pdfFileSpec, int page, boolean newWindow) {
         /* generic creation of object */
         super();
 
         this.pdfFileSpec = pdfFileSpec;
         this.pageReference = page;
+        this.newWindow = newWindow;
     }
 
     /**
@@ -62,13 +69,16 @@ public class PDFGoToRemote extends PDFAction {
      *
      * @param pdfFileSpec the fileSpec associated with the action
      * @param dest a named destination within the remote document
+     * @param newWindow boolean indicating whether the target should be
+     *                  displayed in a new window
      */
-    public PDFGoToRemote(PDFFileSpec pdfFileSpec, String dest) {
+    public PDFGoToRemote(PDFFileSpec pdfFileSpec, String dest, boolean newWindow) {
         /* generic creation of object */
         super();
 
         this.pdfFileSpec = pdfFileSpec;
         this.destination = dest;
+        this.newWindow = newWindow;
     }
 
     /**
@@ -86,12 +96,18 @@ public class PDFGoToRemote extends PDFAction {
     public String toPDFString() {
         StringBuffer sb = new StringBuffer(64);
         sb.append(getObjectID());
-        sb.append("<<\n/S /GoToR\n/F " + pdfFileSpec.referencePDF() + "\n");
+        sb.append("<<\n/S /GoToR\n/F ");
+        sb.append(pdfFileSpec.referencePDF());
+        sb.append("\n");
 
         if (destination != null) {
-            sb.append("/D (" + this.destination + ")");
+            sb.append("/D (").append(this.destination).append(")");
         } else {
-            sb.append("/D [ " + this.pageReference + " /XYZ null null null ]");
+            sb.append("/D [ ").append(this.pageReference).append(" /XYZ null null null ]");
+        }
+
+        if (newWindow) {
+            sb.append("/NewWindow true");
         }
 
         sb.append(" \n>>\nendobj\n");
@@ -142,7 +158,7 @@ public class PDFGoToRemote extends PDFAction {
             }
         }
 
-        return true;
+        return (this.newWindow == remote.newWindow);
     }
 }
 
diff --git a/src/java/org/apache/fop/render/pdf/PDFRenderer.java b/src/java/org/apache/fop/render/pdf/PDFRenderer.java
index 936bf4384..06f5921cc 100644
--- a/src/java/org/apache/fop/render/pdf/PDFRenderer.java
+++ b/src/java/org/apache/fop/render/pdf/PDFRenderer.java
@@ -837,7 +837,7 @@ public class PDFRenderer extends AbstractPathOrientedRenderer {
      * @param value the value
      * @return the formatted value
      */
-    protected static final String format(float value) {
+    protected static String format(float value) {
         return PDFNumber.doubleOut(value);
     }
     
@@ -1134,9 +1134,11 @@ public class PDFRenderer extends AbstractPathOrientedRenderer {
      * (i.e. if the area qualifies as a link target).
      * Otherwise, or if the area has no id, null is returned.
      *
-     * NOTE : area must be on currentPageViewport, otherwise result may be wrong!
+     * <i>NOTE</i>: area must be on currentPageViewport, otherwise result may be wrong!
      *
      * @param area the area for which to return the id
+     * @return the area's id (null if the area has no id or
+     *              other preceding areas have the same id)
      */
     protected String getTargetableID(Area area) {
         String id = (String) area.getTrait(Trait.PROD_ID);
@@ -1374,11 +1376,14 @@ public class PDFRenderer extends AbstractPathOrientedRenderer {
 
         // no INTERNAL_LINK, look for EXTERNAL_LINK
         if (!linkTraitFound) {
-            String extDest = (String) ip.getTrait(Trait.EXTERNAL_LINK);
-            if (extDest != null && extDest.length() > 0) {
-                linkTraitFound = true;
-                if (annotsAllowed) {
-                    action = factory.getExternalAction(extDest);
+            Trait.ExternalLink extLink = (Trait.ExternalLink) ip.getTrait(Trait.EXTERNAL_LINK);
+            if (extLink != null) {
+                String extDest = extLink.getDestination();
+                if (extDest != null && extDest.length() > 0) {
+                    linkTraitFound = true;
+                    if (annotsAllowed) {
+                        action = factory.getExternalAction(extDest, extLink.newWindow());
+                    }
                 }
             }
         }
@@ -1610,7 +1615,7 @@ public class PDFRenderer extends AbstractPathOrientedRenderer {
      * Adds a PDF XObject (a bitmap or form) to the PDF that will later be referenced.
      * @param uri URL of the bitmap
      * @param pos Position of the bitmap
-     * @deprecated Use {@link @putImage(String, Rectangle2D, Map)} instead.
+     * @deprecated Use {@link #putImage(String, Rectangle2D, Map)} instead.
      */
     protected void putImage(String uri, Rectangle2D pos) {
         putImage(uri, pos, null);