Bug 62092 - Text not extracted from grouped text shapes in HSLF

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

src/java/org/apache/poi/sl/draw/DrawTextParagraph.java

@@ -52,6 +52,7 @@ import org.apache.poi.sl.usermodel.TextRun;
 import org.apache.poi.sl.usermodel.TextRun.FieldType;
 import org.apache.poi.sl.usermodel.TextShape;
 import org.apache.poi.sl.usermodel.TextShape.TextDirection;
+import org.apache.poi.util.Internal;
 import org.apache.poi.util.LocaleUtil;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -381,7 +382,8 @@ public class DrawTextParagraph implements Drawable {
         return getRenderableText(tr);
     }
 
-    private String getRenderableText(final TextRun tr) {
+    @Internal
+    public String getRenderableText(final TextRun tr) {
         final String txtSpace = tr.getRawText().replace("\t", tab2space(tr)).replace('\u000b', '\n');
         final Locale loc = LocaleUtil.getUserLocale();
 

src/java/org/apache/poi/sl/extractor/SlideShowExtractor.java

@@ -0,0 +1,307 @@
+package org.apache.poi.sl.extractor;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.poi.POITextExtractor;
+import org.apache.poi.sl.usermodel.Comment;
+import org.apache.poi.sl.usermodel.MasterSheet;
+import org.apache.poi.sl.usermodel.Notes;
+import org.apache.poi.sl.usermodel.ObjectShape;
+import org.apache.poi.sl.usermodel.Placeholder;
+import org.apache.poi.sl.usermodel.PlaceholderDetails;
+import org.apache.poi.sl.usermodel.Shape;
+import org.apache.poi.sl.usermodel.ShapeContainer;
+import org.apache.poi.sl.usermodel.Sheet;
+import org.apache.poi.sl.usermodel.Slide;
+import org.apache.poi.sl.usermodel.SlideShow;
+import org.apache.poi.sl.usermodel.TableCell;
+import org.apache.poi.sl.usermodel.TableShape;
+import org.apache.poi.sl.usermodel.TextParagraph;
+import org.apache.poi.sl.usermodel.TextRun;
+import org.apache.poi.sl.usermodel.TextShape;
+import org.apache.poi.util.LocaleUtil;
+import org.apache.poi.util.POILogFactory;
+import org.apache.poi.util.POILogger;
+
+/**
+ * Common SlideShow extractor
+ *
+ * @since POI 4.0.0
+ */
+public class SlideShowExtractor<
+    S extends Shape<S,P>,
+    P extends TextParagraph<S,P,? extends TextRun>
+> extends POITextExtractor {
+    private static final POILogger LOG = POILogFactory.getLogger(SlideShowExtractor.class);
+
+    private SlideShow<S,P> slideshow;
+
+    private boolean slidesByDefault = true;
+    private boolean notesByDefault;
+    private boolean commentsByDefault;
+    private boolean masterByDefault;
+
+    
+    public SlideShowExtractor(final SlideShow<S,P> slideshow) {
+        setFilesystem(slideshow);
+        this.slideshow = slideshow;
+    }
+
+    /**
+     * Should a call to getText() return slide text? Default is yes
+     */
+    public void setSlidesByDefault(final boolean slidesByDefault) {
+        this.slidesByDefault = slidesByDefault;
+    }
+
+    /**
+     * Should a call to getText() return notes text? Default is no
+     */
+    public void setNotesByDefault(final boolean notesByDefault) {
+        this.notesByDefault = notesByDefault;
+    }
+
+    /**
+     * Should a call to getText() return comments text? Default is no
+     */
+    public void setCommentsByDefault(final boolean commentsByDefault) {
+        this.commentsByDefault = commentsByDefault;
+    }
+
+    /**
+     * Should a call to getText() return text from master? Default is no
+     */
+    public void setMasterByDefault(final boolean masterByDefault) {
+        this.masterByDefault = masterByDefault;
+    }
+
+    @Override
+    public POITextExtractor getMetadataTextExtractor() {
+        return slideshow.getMetadataTextExtractor();
+    }
+
+    /**
+     * Fetches all the slide text from the slideshow, but not the notes, unless
+     * you've called setSlidesByDefault() and setNotesByDefault() to change this
+     */
+    @Override
+    public String getText() {
+        final StringBuilder sb = new StringBuilder();
+        
+        if (masterByDefault) {
+            for (final MasterSheet<S,P> master : slideshow.getSlideMasters()) {
+                for (final Shape<S,P> shape : master) {
+                    if (shape instanceof TextShape) {
+                        final TextShape<S,P> ts = (TextShape<S,P>)shape;
+                        final String text = ts.getText();
+                        if (text == null || text.isEmpty() || "*".equals(text)) {
+                            continue;
+                        }
+                        if (ts.isPlaceholder()) {
+                            // don't bother about boiler plate text on master sheets
+                            LOG.log(POILogger.INFO, "Ignoring boiler plate (placeholder) text on slide master:", text);
+                            continue;
+                        }
+                        sb.append(text);
+                        if (!text.endsWith("\n")) {
+                            sb.append("\n");
+                        }
+
+                    }
+                }
+            }
+        }
+
+        for (final Slide<S, P> slide : slideshow.getSlides()) {
+            sb.append(getText(slide));
+        }
+
+        return sb.toString();
+    }
+
+    public String getText(final Slide<S,P> slide) {
+        final StringBuilder sb = new StringBuilder();
+
+        if (slidesByDefault) {
+            printShapeText(slide, sb);
+        }
+
+        if (commentsByDefault) {
+            printComments(slide, sb);
+        }
+
+        if (notesByDefault) {
+            printNotes(slide, sb);
+        }
+
+        return sb.toString();
+    }
+
+    private String printHeaderReturnFooter(final Sheet<S,P> sheet, final StringBuilder sb) {
+        final Sheet<S, P> m = (sheet instanceof Slide) ? sheet.getMasterSheet() : sheet;
+        final StringBuilder footer = new StringBuilder("\n");
+        addSheetPlaceholderDatails(sheet, Placeholder.HEADER, sb);
+        addSheetPlaceholderDatails(sheet, Placeholder.FOOTER, footer);
+
+        if (masterByDefault) {
+            // write header texts and determine footer text
+            for (Shape<S, P> s : m) {
+                if (!(s instanceof TextShape)) {
+                    continue;
+                }
+                final TextShape<S, P> ts = (TextShape<S, P>) s;
+                final PlaceholderDetails pd = ts.getPlaceholderDetails();
+                if (pd == null || !pd.isVisible()) {
+                    continue;
+                }
+                switch (pd.getPlaceholder()) {
+                    case HEADER:
+                        sb.append(ts.getText());
+                        sb.append('\n');
+                        break;
+                    case SLIDE_NUMBER:
+                        if (sheet instanceof Slide) {
+                            footer.append(ts.getText().replace("‹#›", Integer.toString(((Slide<S, P>) sheet).getSlideNumber() + 1)));
+                            footer.append('\n');
+                        }
+                        break;
+                    case FOOTER:
+                        footer.append(ts.getText());
+                        footer.append('\n');
+                        break;
+                    case DATETIME:
+                        // currently not supported
+                    default:
+                        break;
+                }
+            }
+        }
+
+        return (footer.length() > 1) ? footer.toString() : "";
+    }
+
+    private void addSheetPlaceholderDatails(final Sheet<S,P> sheet, final Placeholder placeholder, final StringBuilder sb) {
+        final PlaceholderDetails headerPD = sheet.getPlaceholderDetails(placeholder);
+        if (headerPD == null) {
+            return;
+        }
+        final String headerStr = headerPD.getText();
+        if (headerStr == null) {
+            return;
+        }
+        sb.append(headerStr);
+    }
+
+    private void printShapeText(final Sheet<S,P> sheet, final StringBuilder sb) {
+        final String footer = printHeaderReturnFooter(sheet, sb);
+        printShapeText((ShapeContainer<S,P>)sheet, sb);
+        sb.append(footer);
+    }
+
+    @SuppressWarnings("unchecked")
+    private void printShapeText(final ShapeContainer<S,P> container, final StringBuilder sb) {
+        for (Shape<S,P> shape : container) {
+            if (shape instanceof TextShape) {
+                printShapeText((TextShape<S,P>)shape, sb);
+            } else if (shape instanceof TableShape) {
+                printShapeText((TableShape<S,P>)shape, sb);
+            } else if (shape instanceof ShapeContainer) {
+                printShapeText((ShapeContainer<S,P>)shape, sb);
+            }
+        }
+    }
+
+    private void printShapeText(final TextShape<S,P> shape, final StringBuilder sb) {
+        final List<P> paraList = shape.getTextParagraphs();
+        if (paraList.isEmpty()) {
+            sb.append('\n');
+            return;
+        }
+        for (final P para : paraList) {
+            final int oldLen = sb.length();
+            for (final TextRun tr : para) {
+                final String str = tr.getRawText().replace("\r", "");
+                final String newStr;
+                switch (tr.getTextCap()) {
+                    case ALL:
+                        newStr = str.toUpperCase(LocaleUtil.getUserLocale());
+                        break;
+                    case SMALL:
+                        newStr = str.toLowerCase(LocaleUtil.getUserLocale());
+                        break;
+                    default:
+                    case NONE:
+                        newStr = str;
+                        break;
+                }
+                sb.append(newStr);
+            }
+            sb.append('\n');
+        }
+    }
+
+    @SuppressWarnings("Duplicates")
+    private void printShapeText(final TableShape<S,P> shape, final StringBuilder sb) {
+        final int nrows = shape.getNumberOfRows();
+        final int ncols = shape.getNumberOfColumns();
+        for (int row = 0; row < nrows; row++){
+            for (int col = 0; col < ncols; col++){
+                TableCell<S, P> cell = shape.getCell(row, col);
+                //defensive null checks; don't know if they're necessary
+                if (cell != null){
+                    String txt = cell.getText();
+                    txt = (txt == null) ? "" : txt;
+                    sb.append(txt);
+                    if (col < ncols-1){
+                        sb.append('\t');
+                    }
+                }
+            }
+            sb.append('\n');
+        }
+    }
+
+    private void printComments(final Slide<S,P> slide, final StringBuilder sb) {
+        for (final Comment comment : slide.getComments()) {
+            sb.append(comment.getAuthor());
+            sb.append(" - ");
+            sb.append(comment.getText());
+            sb.append("\n");
+        }
+    }
+
+    private void printNotes(final Slide<S,P> slide, final StringBuilder sb) {
+        final Notes<S, P> notes = slide.getNotes();
+        if (notes == null) {
+            return;
+        }
+
+        final String footer = printHeaderReturnFooter(notes, sb);
+
+        printShapeText(notes, sb);
+
+        sb.append(footer);
+    }
+
+    public List<? extends ObjectShape<S,P>> getOLEShapes() {
+        final List<ObjectShape<S,P>> oleShapes = new ArrayList<>();
+        
+        for (final Slide<S,P> slide : slideshow.getSlides()) {
+            addOLEShapes(oleShapes, slide);
+        }
+        
+        return oleShapes;
+    }
+    
+    @SuppressWarnings("unchecked")
+    private void addOLEShapes(final List<ObjectShape<S,P>> oleShapes, ShapeContainer<S,P> container) {
+        for (Shape<S,P> shape : container) {
+            if (shape instanceof ShapeContainer) {
+                addOLEShapes(oleShapes, (ShapeContainer<S,P>)shape);
+            } else if (shape instanceof ObjectShape) {
+                oleShapes.add((ObjectShape<S,P>)shape);
+            }
+        }
+    }
+}

src/java/org/apache/poi/sl/usermodel/Comment.java

@@ -0,0 +1,85 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.sl.usermodel;
+
+import java.awt.geom.Point2D;
+import java.util.Date;
+
+/**
+ * Common interface for comments
+ *
+ * @since POI 4.0.0
+ */
+public interface Comment {
+    /**
+     * Get the Author of this comment
+     */
+    String getAuthor();
+
+    /**
+     * Set the Author of this comment.
+     * if the author wasn't registered before, create a new entry
+     */
+    void setAuthor(String author);
+
+    /**
+     * Get the Author's Initials of this comment
+     */
+    String getAuthorInitials();
+
+    /**
+     * Set the Author's Initials of this comment.
+     * if the author wasn't registered before via {@link #setAuthor(String)}
+     * this has no effect
+     */
+    void setAuthorInitials(String initials);
+
+    /**
+     * Get the text of this comment
+     */
+    String getText();
+
+    /**
+     * Set the text of this comment
+     */
+    void setText(String text);
+
+    /**
+     * Gets the date the comment was made.
+     * @return the comment date.
+     */
+    Date getDate();
+
+    /**
+     * Sets the date the comment was made.
+     * @param date the comment date.
+     */
+    void setDate(Date date);
+
+    /**
+     * Gets the offset of the comment on the page.
+     * @return the offset.
+     */
+    Point2D getOffset();
+
+    /**
+     * Sets the offset of the comment on the page.
+     * @param offset the offset.
+     */
+    void setOffset(Point2D offset);
+}

src/java/org/apache/poi/sl/usermodel/PlaceholderDetails.java

@@ -0,0 +1,69 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.sl.usermodel;
+
+
+/**
+ * Extended details about placholders
+ *
+ * @since POI 4.0.0
+ */
+public interface PlaceholderDetails {
+    enum PlaceholderSize {
+        quarter, half, full;
+    }
+    
+    Placeholder getPlaceholder();
+
+    /**
+     * Specifies that the corresponding shape should be represented by the generating application
+     * as a placeholder. When a shape is considered a placeholder by the generating application
+     * it can have special properties to alert the user that they may enter content into the shape.
+     * Different types of placeholders are allowed and can be specified by using the placeholder
+     * type attribute for this element
+     *
+     * @param placeholder The shape to use as placeholder or null if no placeholder should be set.
+     */
+    void setPlaceholder(Placeholder placeholder);
+    
+    boolean isVisible();
+    
+    void setVisible(boolean isVisible);
+    
+    PlaceholderSize getSize();
+    
+    void setSize(PlaceholderSize size);
+
+    /**
+     * If the placeholder shape or object stores text, this text is returned otherwise {@code null}.
+     *
+     * @return the text of the shape / placeholder
+     *
+     * @since POI 4.0.0
+     */
+    String getText();
+
+    /**
+     * If the placeholder shape or object stores text, the given text is stored otherwise this is a no-op.
+     *
+     * @param text the placeholder text
+     *
+     * @since POI 4.0.0
+     */
+    void setText(String text);
+}

src/java/org/apache/poi/sl/usermodel/Sheet.java

@@ -46,4 +46,16 @@ public interface Sheet<
 	 * @param graphics
 	 */
 	void draw(Graphics2D graphics);
+
+	/**
+	 * Get the placeholder details for the given placeholder type. Not all placeholders are also shapes -
+	 * this is especially true for old HSLF slideshows, which notes have header/footers elements which
+	 * aren't shapes.
+	 *
+	 * @param placeholder the placeholder type
+	 * @return the placeholder details or {@code null}, if the placeholder isn't contained in the sheet
+	 *
+	 * @since POI 4.0.0
+	 */
+	PlaceholderDetails getPlaceholderDetails(Placeholder placeholder);
 }

src/java/org/apache/poi/sl/usermodel/SimpleShape.java

@@ -65,6 +65,24 @@ public interface SimpleShape<
      */
     void setPlaceholder(Placeholder placeholder);
 
+    /**
+     * @return an accessor for placeholder details
+     *
+     * @since POI 4.0.0
+     */
+    PlaceholderDetails getPlaceholderDetails();
+
+    /**
+     * Checks if the shape is a placeholder.
+     * (placeholders aren't normal shapes, they are visible only in the Edit Master mode)
+     *
+     * @return {@code true} if the shape is a placeholder
+     * 
+     * @since POI 4.0.0
+     */
+    boolean isPlaceholder();
+    
+    
 	Shadow<S,P> getShadow();
 
     /**

src/java/org/apache/poi/sl/usermodel/Slide.java

@@ -17,6 +17,8 @@
 
 package org.apache.poi.sl.usermodel;
 
+import java.util.List;
+
 public interface Slide<
     S extends Shape<S,P>,
     P extends TextParagraph<S,P,? extends TextRun>
@@ -48,7 +50,7 @@ public interface Slide<
      * whereas in HSLF they are activated via a HeadersFooter configuration.
      * This method is used to generalize that handling.
      *
-     * @param placeholder
+     * @param placeholder the placeholder type
      * @return {@code true} if the placeholder should be displayed/rendered
      * @since POI 3.16-beta2
      */
@@ -69,4 +71,9 @@ public interface Slide<
      * @since POI 4.0.0
      */
     boolean isHidden();
+
+    /**
+     * @return the comment(s) for this slide
+     */
+    List<? extends Comment> getComments();
 }

src/java/org/apache/poi/sl/usermodel/SlideShow.java

@@ -25,6 +25,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.List;
 
+import org.apache.poi.POITextExtractor;
 import org.apache.poi.sl.usermodel.PictureData.PictureType;
 
 public interface SlideShow<
@@ -118,4 +119,11 @@ public interface SlideShow<
      *             OutputStream
      */
     void write(OutputStream out) throws IOException;
+
+    /**
+     * @return an extractor for the slideshow metadata
+     * 
+     * @since POI 4.0.0
+     */
+    POITextExtractor getMetadataTextExtractor();
 }

src/ooxml/java/org/apache/poi/xslf/extractor/XSLFPowerPointExtractor.java

@@ -21,50 +21,54 @@ import java.io.IOException;
 import org.apache.poi.POIXMLTextExtractor;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
+import org.apache.poi.sl.extractor.SlideShowExtractor;
+import org.apache.poi.util.Removal;
 import org.apache.poi.xslf.usermodel.XMLSlideShow;
-import org.apache.poi.xslf.usermodel.XSLFCommentAuthors;
-import org.apache.poi.xslf.usermodel.XSLFComments;
-import org.apache.poi.xslf.usermodel.XSLFNotes;
 import org.apache.poi.xslf.usermodel.XSLFRelation;
 import org.apache.poi.xslf.usermodel.XSLFShape;
-import org.apache.poi.xslf.usermodel.XSLFShapeContainer;
 import org.apache.poi.xslf.usermodel.XSLFSlide;
-import org.apache.poi.xslf.usermodel.XSLFSlideLayout;
-import org.apache.poi.xslf.usermodel.XSLFSlideMaster;
 import org.apache.poi.xslf.usermodel.XSLFSlideShow;
-import org.apache.poi.xslf.usermodel.XSLFTable;
+import org.apache.poi.xslf.usermodel.XSLFTextParagraph;
-import org.apache.poi.xslf.usermodel.XSLFTableCell;
-import org.apache.poi.xslf.usermodel.XSLFTableRow;
-import org.apache.poi.xslf.usermodel.XSLFTextShape;
 import org.apache.xmlbeans.XmlException;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTComment;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTCommentAuthor;
 
+/**
+ * Extractor for XSLF SlideShows
+ *
+ * @deprecated use {@link SlideShowExtractor}
+ */
+@Deprecated
+@Removal(version="4.2.0")
 public class XSLFPowerPointExtractor extends POIXMLTextExtractor {
-   public static final XSLFRelation[] SUPPORTED_TYPES = new XSLFRelation[] {
+    public static final XSLFRelation[] SUPPORTED_TYPES = new XSLFRelation[]{
             XSLFRelation.MAIN, XSLFRelation.MACRO, XSLFRelation.MACRO_TEMPLATE,
             XSLFRelation.PRESENTATIONML, XSLFRelation.PRESENTATIONML_TEMPLATE,
             XSLFRelation.PRESENTATION_MACRO
     };
 
-	private XMLSlideShow slideshow;
+    private final SlideShowExtractor<XSLFShape, XSLFTextParagraph> delegate;
+
+
     private boolean slidesByDefault = true;
     private boolean notesByDefault;
+    private boolean commentsByDefault;
     private boolean masterByDefault;
 
-	public XSLFPowerPointExtractor(XMLSlideShow slideshow) {
+    @SuppressWarnings("WeakerAccess")
-		super(slideshow);
+    public XSLFPowerPointExtractor(XMLSlideShow slideShow) {
-		this.slideshow = slideshow;
+        super(slideShow);
+        delegate = new SlideShowExtractor<>(slideShow);
     }
-	public XSLFPowerPointExtractor(XSLFSlideShow slideshow) throws XmlException, IOException {
+
-		this(new XMLSlideShow(slideshow.getPackage()));
+    public XSLFPowerPointExtractor(XSLFSlideShow slideShow) {
+        this(new XMLSlideShow(slideShow.getPackage()));
     }
+
     public XSLFPowerPointExtractor(OPCPackage container) throws XmlException, OpenXML4JException, IOException {
         this(new XSLFSlideShow(container));
     }
 
     public static void main(String[] args) throws Exception {
-		if(args.length < 1) {
+        if (args.length < 1) {
             System.err.println("Use:");
             System.err.println("  XSLFPowerPointExtractor <filename.pptx>");
             System.exit(1);
@@ -80,22 +84,38 @@ public class XSLFPowerPointExtractor extends POIXMLTextExtractor {
      * Should a call to getText() return slide text?
      * Default is yes
      */
-	public void setSlidesByDefault(boolean slidesByDefault) {
+    @SuppressWarnings("WeakerAccess")
+    public void setSlidesByDefault(final boolean slidesByDefault) {
         this.slidesByDefault = slidesByDefault;
+        delegate.setSlidesByDefault(slidesByDefault);
     }
+
     /**
      * Should a call to getText() return notes text?
      * Default is no
      */
-	public void setNotesByDefault(boolean notesByDefault) {
+    @SuppressWarnings("WeakerAccess")
+    public void setNotesByDefault(final boolean notesByDefault) {
         this.notesByDefault = notesByDefault;
+        delegate.setNotesByDefault(notesByDefault);
+    }
+
+    /**
+     * Should a call to getText() return comments text? Default is no
+     */
+    @SuppressWarnings({"WeakerAccess", "unused"})
+    public void setCommentsByDefault(final boolean commentsByDefault) {
+        this.commentsByDefault = commentsByDefault;
+        delegate.setCommentsByDefault(commentsByDefault);
     }
 
     /**
      * Should a call to getText() return text from master? Default is no
      */
-   public void setMasterByDefault(boolean masterByDefault) {
+    @SuppressWarnings("WeakerAccess")
+    public void setMasterByDefault(final boolean masterByDefault) {
         this.masterByDefault = masterByDefault;
+        delegate.setMasterByDefault(masterByDefault);
     }
 
     /**
@@ -103,16 +123,17 @@ public class XSLFPowerPointExtractor extends POIXMLTextExtractor {
      */
     @Override
     public String getText() {
-		return getText(slidesByDefault, notesByDefault);
+        return delegate.getText();
     }
 
     /**
      * Gets the requested text from the file
+     *
      * @param slideText Should we retrieve text from slides?
      * @param notesText Should we retrieve text from notes?
      */
-   public String getText(boolean slideText, boolean notesText) {
+    public String getText(final boolean slideText, final boolean notesText) {
-      return getText(slideText, notesText, masterByDefault);
+        return getText(slideText, notesText, commentsByDefault, masterByDefault);
     }
 
     /**
@@ -121,17 +142,36 @@ public class XSLFPowerPointExtractor extends POIXMLTextExtractor {
      * @param slideText  Should we retrieve text from slides?
      * @param notesText  Should we retrieve text from notes?
      * @param masterText Should we retrieve text from master slides?
-    * 
      * @return the extracted text
      */
     public String getText(boolean slideText, boolean notesText, boolean masterText) {
-      StringBuilder text = new StringBuilder();
+        return getText(slideText, notesText, commentsByDefault, masterText);
-
-      for (XSLFSlide slide : slideshow.getSlides()) {
-          text.append(getText(slide, slideText, notesText, masterText));
     }
 
-      return text.toString();
+
+    /**
+     * Gets the requested text from the file
+     *
+     * @param slideText   Should we retrieve text from slides?
+     * @param notesText   Should we retrieve text from notes?
+     * @param commentText Should we retrieve text from comments?
+     * @param masterText  Should we retrieve text from master slides?
+     * @return the extracted text
+     */
+    @SuppressWarnings("Duplicates")
+    public String getText(boolean slideText, boolean notesText, boolean commentText, boolean masterText) {
+        delegate.setSlidesByDefault(slideText);
+        delegate.setNotesByDefault(notesText);
+        delegate.setCommentsByDefault(commentText);
+        delegate.setMasterByDefault(masterText);
+        try {
+            return delegate.getText();
+        } finally {
+            delegate.setSlidesByDefault(slidesByDefault);
+            delegate.setNotesByDefault(notesByDefault);
+            delegate.setCommentsByDefault(commentsByDefault);
+            delegate.setMasterByDefault(masterByDefault);
+        }
     }
 
     /**
@@ -141,82 +181,28 @@ public class XSLFPowerPointExtractor extends POIXMLTextExtractor {
      * @param slideText   Should we retrieve text from slides?
      * @param notesText   Should we retrieve text from notes?
      * @param masterText  Should we retrieve text from master slides?
-    * 
      * @return the extracted text
      */
     public static String getText(XSLFSlide slide, boolean slideText, boolean notesText, boolean masterText) {
-       StringBuilder text = new StringBuilder();
+        return getText(slide, slideText, notesText, false, masterText);
-
-       XSLFCommentAuthors commentAuthors = slide.getSlideShow().getCommentAuthors();
-
-       XSLFNotes notes = slide.getNotes();
-       XSLFComments comments = slide.getComments();
-       XSLFSlideLayout layout = slide.getSlideLayout();
-       XSLFSlideMaster master = layout.getSlideMaster();
-
-       // TODO Do the slide's name
-       // (Stored in docProps/app.xml)
-
-       // Do the slide's text if requested
-       if (slideText) {
-          extractText(slide, false, text);
-          
-          // If requested, get text from the master and it's layout 
-          if(masterText) {
-             assert (layout != null);
-             extractText(layout, true, text);
-             assert (master != null);
-             extractText(master, true, text);
     }
 
-          // If the slide has comments, do those too
+    /**
-          if (comments != null) {
+     * Gets the requested text from the slide
-             for (CTComment comment : comments.getCTCommentsList().getCmArray()) {
+     *
-                // Do the author if we can
+     * @param slide       the slide to retrieve the text from
-                if (commentAuthors != null) {
+     * @param slideText   Should we retrieve text from slides?
-                   CTCommentAuthor author = commentAuthors.getAuthorById(comment.getAuthorId());
+     * @param notesText   Should we retrieve text from notes?
-                   if(author != null) {
+     * @param commentText Should we retrieve text from comments?
-                      text.append(author.getName() + ": ");
+     * @param masterText  Should we retrieve text from master slides?
-                   }
+     * @return the extracted text
-                }
+     */
-                
+    public static String getText(XSLFSlide slide, boolean slideText, boolean notesText, boolean commentText, boolean masterText) {
-                // Then the comment text, with a new line afterwards
+        final SlideShowExtractor<XSLFShape, XSLFTextParagraph> ex = new SlideShowExtractor<>(slide.getSlideShow());
-                text.append(comment.getText());
+        ex.setSlidesByDefault(slideText);
-                text.append("\n");
+        ex.setNotesByDefault(notesText);
-             }
+        ex.setCommentsByDefault(commentText);
-          }
+        ex.setMasterByDefault(masterText);
-       }
+        return ex.getText(slide);
-
-       // Do the notes if requested
-       if (notesText && notes != null) {
-          extractText(notes, false, text);
-       }
-       
-       return text.toString();
-   }
-   
-    private static void extractText(XSLFShapeContainer data, boolean skipPlaceholders, StringBuilder text) {
-       for (XSLFShape s : data) {
-           if (s instanceof XSLFShapeContainer) {
-               extractText((XSLFShapeContainer)s, skipPlaceholders, text);
-           } else if (s instanceof XSLFTextShape) {
-               XSLFTextShape ts = (XSLFTextShape)s;
-               // Skip non-customised placeholder text
-               if (!(skipPlaceholders && ts.isPlaceholder())) {
-                   text.append(ts.getText());
-                   text.append("\n");
-               }
-           } else if (s instanceof XSLFTable) {
-               XSLFTable ts = (XSLFTable)s;
-               // Skip non-customised placeholder text
-               for (XSLFTableRow r : ts) {
-                   for (XSLFTableCell c : r) {
-                       text.append(c.getText());
-                       text.append("\t");
-                   }
-                   text.append("\n");
-               }
-           }
-       }
     }
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

@@ -35,10 +35,10 @@ import java.util.regex.Pattern;
 import org.apache.poi.POIXMLDocument;
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.POIXMLException;
+import org.apache.poi.POIXMLPropertiesTextExtractor;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackagePart;
-import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.sl.usermodel.MasterSheet;
 import org.apache.poi.sl.usermodel.PictureData.PictureType;
 import org.apache.poi.sl.usermodel.Resources;
@@ -626,4 +626,9 @@ public class XMLSlideShow extends POIXMLDocument
         // TODO: implement!
         throw new UnsupportedOperationException();
     }
+    
+    @Override
+    public POIXMLPropertiesTextExtractor getMetadataTextExtractor() {
+        return new POIXMLPropertiesTextExtractor(this);
+    }
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFComment.java

@@ -0,0 +1,126 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xslf.usermodel;
+
+import java.awt.geom.Point2D;
+import java.util.Calendar;
+import java.util.Date;
+
+import org.apache.poi.sl.usermodel.Comment;
+import org.apache.poi.util.LocaleUtil;
+import org.apache.poi.util.Units;
+import org.openxmlformats.schemas.drawingml.x2006.main.CTPoint2D;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTComment;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTCommentAuthor;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTCommentAuthorList;
+
+/**
+ * XSLF Comment
+ *
+ * @since POI 4.0.0
+ */
+public class XSLFComment implements Comment {
+
+    final CTComment comment;
+    final XSLFCommentAuthors authors;
+
+    XSLFComment(final CTComment comment, final XSLFCommentAuthors authors) {
+        this.comment = comment;
+        this.authors = authors;
+    }
+
+    @Override
+    public String getAuthor() {
+        return authors.getAuthorById(comment.getAuthorId()).getName();
+    }
+
+    @Override
+    public void setAuthor(final String author) {
+        if (author == null) {
+            throw new IllegalArgumentException("author must not be null");
+        }
+        final CTCommentAuthorList list = authors.getCTCommentAuthorsList();
+        long maxId = -1;
+        for (final CTCommentAuthor aut : list.getCmAuthorArray()) {
+            maxId = Math.max(aut.getId(), maxId);
+            if (author.equals(aut.getName())) {
+                comment.setAuthorId(aut.getId());
+                return;
+            }
+        }
+        // author not found -> add new author
+        final CTCommentAuthor newAuthor = list.addNewCmAuthor();
+        newAuthor.setName(author);
+        newAuthor.setId(maxId+1);
+        newAuthor.setInitials(author.replaceAll(	"\\s*(\\w)\\S*", "$1").toUpperCase(LocaleUtil.getUserLocale()));
+        comment.setAuthorId(maxId+1);
+    }
+
+    @Override
+    public String getAuthorInitials() {
+        final CTCommentAuthor aut = authors.getAuthorById(comment.getAuthorId());
+        return aut == null ? null : aut.getInitials();
+    }
+
+    @Override
+    public void setAuthorInitials(final String initials) {
+        final CTCommentAuthor aut = authors.getAuthorById(comment.getAuthorId());
+        if (aut != null) {
+            aut.setInitials(initials);
+        }
+    }
+
+    @Override
+    public String getText() {
+        return comment.getText();
+    }
+
+    @Override
+    public void setText(final String text) {
+        comment.setText(text);
+    }
+
+    @Override
+    public Date getDate() {
+        final Calendar cal = comment.getDt();
+        return (cal == null) ? null : cal.getTime();
+    }
+
+    @Override
+    public void setDate(final Date date) {
+        final Calendar cal = LocaleUtil.getLocaleCalendar();
+        cal.setTime(date);
+        comment.setDt(cal);
+    }
+
+    @Override
+    public Point2D getOffset() {
+        final CTPoint2D pos = comment.getPos();
+        return new Point2D.Double(Units.toPoints(pos.getX()), Units.toPoints(pos.getY()));
+    }
+
+    @Override
+    public void setOffset(final Point2D offset) {
+        CTPoint2D pos = comment.getPos();
+        if (pos == null) {
+            pos = comment.addNewPos();
+        }
+        pos.setX(Units.toEMU(offset.getX()));
+        pos.setY(Units.toEMU(offset.getY()));
+    }
+}

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFComments.java

@@ -31,15 +31,13 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CmLstDocument;
 
 @Beta
 public class XSLFComments extends POIXMLDocumentPart {
-    private final CTCommentList _comments;
+    private final CmLstDocument doc;
 
     /**
      * Create a new set of slide comments
      */
     XSLFComments() {
-       super();
+        doc = CmLstDocument.Factory.newInstance();
-       CmLstDocument doc = CmLstDocument.Factory.newInstance();
-       _comments = doc.addNewCmLst();
     }
 
     /**
@@ -47,26 +45,23 @@ public class XSLFComments extends POIXMLDocumentPart {
      *
      * @param part the package part holding the comments data,
      *             the content type must be <code>application/vnd.openxmlformats-officedocument.comments+xml</code>
-     * 
      * @since POI 3.14-Beta1
      */
     XSLFComments(PackagePart part) throws IOException, XmlException {
         super(part);
 
-        CmLstDocument doc =
+        doc = CmLstDocument.Factory.parse(getPackagePart().getInputStream(), DEFAULT_XML_OPTIONS);
-           CmLstDocument.Factory.parse(getPackagePart().getInputStream(), DEFAULT_XML_OPTIONS);
-        _comments = doc.getCmLst();
     }
 
     public CTCommentList getCTCommentsList() {
-       return _comments;
+        return doc.getCmLst();
     }
 
     public int getNumberOfComments() {
-       return _comments.sizeOfCmArray();
+        return doc.getCmLst().sizeOfCmArray();
     }
 
     public CTComment getCommentAt(int pos) {
-       return _comments.getCmArray(pos);
+        return doc.getCmLst().getCmArray(pos);
     }
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFNotesMaster.java

@@ -51,7 +51,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.NotesMasterDocument;
  public class XSLFNotesMaster extends XSLFSheet
      implements MasterSheet<XSLFShape,XSLFTextParagraph> {
 	 private CTNotesMaster _slide;
-     private XSLFTheme _theme;
 
     XSLFNotesMaster() {
         super();
@@ -101,20 +100,14 @@ import org.openxmlformats.schemas.presentationml.x2006.main.NotesMasterDocument;
         return null;
     }
 
+
     @Override
-    public XSLFTheme getTheme() {
+    boolean isSupportTheme() {
-        if (_theme == null) {
+        return true;
-            for (POIXMLDocumentPart p : getRelations()) {
-                if (p instanceof XSLFTheme) {
-                    _theme = (XSLFTheme) p;
-                    CTColorMapping cmap = _slide.getClrMap();
-                    if (cmap != null) {
-                        _theme.initColorMap(cmap);
     }
-                    break;
+
-                }
+    @Override
-            }
+    CTColorMapping getColorMapping() {
-        }
+        return _slide.getClrMap();
-        return _theme;
     }
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPlaceholderDetails.java

@@ -0,0 +1,203 @@
+package org.apache.poi.xslf.usermodel;
+
+import static org.apache.poi.xslf.usermodel.XSLFShape.PML_NS;
+
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+import org.apache.poi.sl.usermodel.MasterSheet;
+import org.apache.poi.sl.usermodel.Placeholder;
+import org.apache.poi.sl.usermodel.PlaceholderDetails;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTApplicationNonVisualDrawingProps;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTHeaderFooter;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTNotesMaster;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideMaster;
+import org.openxmlformats.schemas.presentationml.x2006.main.STPlaceholderSize;
+import org.openxmlformats.schemas.presentationml.x2006.main.STPlaceholderType;
+
+/**
+ * XSLF Placeholder Details
+ *
+ * @since POI 4.0.0
+ */
+public class XSLFPlaceholderDetails implements PlaceholderDetails {
+
+    private final XSLFShape shape;
+    private CTPlaceholder _ph;
+
+    XSLFPlaceholderDetails(final XSLFShape shape) {
+        this.shape = shape;
+    }
+
+    @Override
+    public Placeholder getPlaceholder() {
+        final CTPlaceholder ph = getCTPlaceholder(false);
+        if (ph == null || !(ph.isSetType() || ph.isSetIdx())) {
+            return null;
+        }
+        return Placeholder.lookupOoxml(ph.getType().intValue());
+    }
+
+    @Override
+    public void setPlaceholder(final Placeholder placeholder) {
+        CTPlaceholder ph = getCTPlaceholder(placeholder != null);
+        if (ph != null) {
+            if (placeholder != null) {
+                ph.setType(STPlaceholderType.Enum.forInt(placeholder.ooxmlId));
+            } else {
+                getNvProps().unsetPh();
+            }
+        }
+    }
+
+    @Override
+    public boolean isVisible() {
+        final CTPlaceholder ph = getCTPlaceholder(false);
+        if (ph == null || !ph.isSetType()) {
+            return true;
+        }
+        final CTHeaderFooter hf = getHeaderFooter(false);
+        if (hf == null) {
+            return false;
+        }
+
+        final Placeholder pl = Placeholder.lookupOoxml(ph.getType().intValue());
+        if (pl == null) {
+            return true;
+        }
+        switch (pl) {
+            case DATETIME:
+                return !hf.isSetDt() || hf.getDt();
+            case FOOTER:
+                return !hf.isSetFtr() || hf.getFtr();
+            case HEADER:
+                return !hf.isSetHdr() || hf.getHdr();
+            case SLIDE_NUMBER:
+                return !hf.isSetSldNum() || hf.getSldNum();
+            default:
+                return true;
+        }
+    }
+
+    @Override
+    public void setVisible(final boolean isVisible) {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return;
+        }
+        final Function<CTHeaderFooter,Consumer<Boolean>> fun;
+        switch (ph) {
+            case DATETIME:
+                fun = (hf) -> hf::setDt;
+                break;
+            case FOOTER:
+                fun = (hf) -> hf::setFtr;
+                break;
+            case HEADER:
+                fun = (hf) -> hf::setHdr;
+                break;
+            case SLIDE_NUMBER:
+                fun = (hf) -> hf::setSldNum;
+                break;
+            default:
+                return;
+        }
+        // only create a header, if we need to, i.e. the placeholder type is eligible
+        final CTHeaderFooter hf = getHeaderFooter(true);
+        if (hf == null) {
+            return;
+        }
+        fun.apply(hf).accept(isVisible);
+    }
+
+    @Override
+    public PlaceholderSize getSize() {
+        final CTPlaceholder ph = getCTPlaceholder(false);
+        if (ph == null || !ph.isSetSz()) {
+            return null;
+        }
+        switch (ph.getSz().intValue()) {
+            case STPlaceholderSize.INT_FULL:
+                return PlaceholderSize.full;
+            case STPlaceholderSize.INT_HALF:
+                return PlaceholderSize.half;
+            case STPlaceholderSize.INT_QUARTER:
+                return PlaceholderSize.quarter;
+            default:
+                return null;
+        }
+    }
+
+    @Override
+    public void setSize(final PlaceholderSize size) {
+        final CTPlaceholder ph = getCTPlaceholder(false);
+        if (ph == null) {
+            return;
+        }
+        if (size == null) {
+            ph.unsetSz();
+            return;
+        }
+        switch (size) {
+            case full:
+                ph.setSz(STPlaceholderSize.FULL);
+                break;
+            case half:
+                ph.setSz(STPlaceholderSize.HALF);
+                break;
+            case quarter:
+                ph.setSz(STPlaceholderSize.QUARTER);
+                break;
+        }
+    }
+
+    /**
+     * Gets or creates a new placeholder element
+     *
+     * @param create if {@code true} creates the element if it hasn't existed before
+     * @return the placeholder or {@code null} if the shape doesn't support placeholders
+     */
+    CTPlaceholder getCTPlaceholder(final boolean create) {
+        if (_ph != null) {
+            return _ph;
+        }
+
+        final CTApplicationNonVisualDrawingProps nv = getNvProps();
+        if (nv == null) {
+            // shape doesn't support CTApplicationNonVisualDrawingProps
+            return null;
+        }
+
+        _ph = (nv.isSetPh() || !create) ? nv.getPh() : nv.addNewPh();
+        return _ph;
+    }
+
+    private CTApplicationNonVisualDrawingProps getNvProps() {
+        final String xquery = "declare namespace p='" + PML_NS + "' .//*/p:nvPr";
+        return shape.selectProperty(CTApplicationNonVisualDrawingProps.class, xquery);
+    }
+
+    private CTHeaderFooter getHeaderFooter(final boolean create) {
+        final XSLFSheet sheet = shape.getSheet();
+        final XSLFSheet master = (sheet instanceof MasterSheet && !(sheet instanceof XSLFSlideLayout)) ? sheet : (XSLFSheet)sheet.getMasterSheet();
+        if (master instanceof XSLFSlideMaster) {
+            final CTSlideMaster ct = ((XSLFSlideMaster) master).getXmlObject();
+            return (ct.isSetHf() || !create) ? ct.getHf() : ct.addNewHf();
+        } else if (master instanceof  XSLFNotesMaster) {
+            final CTNotesMaster ct = ((XSLFNotesMaster) master).getXmlObject();
+            return (ct.isSetHf() || !create) ? ct.getHf() : ct.addNewHf();
+        } else {
+            return null;
+        }
+    }
+
+    @Override
+    public String getText() {
+        return null;
+    }
+
+    @Override
+    public void setText(String text) {
+    }
+}

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java

@@ -38,7 +38,9 @@ import org.apache.poi.sl.usermodel.PaintStyle.GradientPaint;
 import org.apache.poi.sl.usermodel.PaintStyle.TexturePaint;
 import org.apache.poi.sl.usermodel.PlaceableShape;
 import org.apache.poi.sl.usermodel.Placeholder;
+import org.apache.poi.sl.usermodel.PlaceholderDetails;
 import org.apache.poi.sl.usermodel.Shape;
+import org.apache.poi.sl.usermodel.SimpleShape;
 import org.apache.poi.util.Beta;
 import org.apache.poi.util.Internal;
 import org.apache.poi.xslf.model.PropertyFetcher;
@@ -58,7 +60,6 @@ import org.openxmlformats.schemas.drawingml.x2006.main.CTSolidColorFillPropertie
 import org.openxmlformats.schemas.drawingml.x2006.main.CTStyleMatrix;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTStyleMatrixReference;
 import org.openxmlformats.schemas.drawingml.x2006.main.STPathShadeType;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTApplicationNonVisualDrawingProps;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTBackgroundProperties;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTShape;
@@ -69,7 +70,7 @@ import org.openxmlformats.schemas.presentationml.x2006.main.STPlaceholderType;
  */
 @Beta
 public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
-    protected static final String PML_NS = "http://schemas.openxmlformats.org/presentationml/2006/main";
+    static final String PML_NS = "http://schemas.openxmlformats.org/presentationml/2006/main";
     
     private final XmlObject _shape;
     private final XSLFSheet _sheet;
@@ -77,7 +78,6 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
 
     private CTShapeStyle _spStyle;
     private CTNonVisualDrawingProps _nvPr;
-    private CTPlaceholder _ph;
 
     protected XSLFShape(XmlObject shape, XSLFSheet sheet) {
         _shape = shape;
@@ -239,43 +239,30 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
         return child;
     }
 
-    protected CTPlaceholder getCTPlaceholder() {
+    public boolean isPlaceholder() {
-        if (_ph == null) {
+        return getPlaceholderDetails().getCTPlaceholder(false) != null;
-            String xquery = "declare namespace p='"+PML_NS+"' .//*/p:nvPr/p:ph";
-            _ph = selectProperty(CTPlaceholder.class, xquery);
-        }
-        return _ph;
-    }
-
-    public Placeholder getPlaceholder() {
-        CTPlaceholder ph = getCTPlaceholder();
-        if (ph == null || !(ph.isSetType() || ph.isSetIdx())) {
-            return null;
-        }
-        return Placeholder.lookupOoxml(ph.getType().intValue());
     }
 
     /**
-     * Specifies that the corresponding shape should be represented by the generating application
+     * @see PlaceholderDetails#getPlaceholder()
-     * as a placeholder. When a shape is considered a placeholder by the generating application
-     * it can have special properties to alert the user that they may enter content into the shape.
-     * Different types of placeholders are allowed and can be specified by using the placeholder
-     * type attribute for this element
-     *
-     * @param placeholder The shape to use as placeholder or null if no placeholder should be set.
      */
-    protected void setPlaceholder(Placeholder placeholder) {
+    public Placeholder getPlaceholder() {
-        String xquery = "declare namespace p='"+PML_NS+"' .//*/p:nvPr";
+        return getPlaceholderDetails().getPlaceholder();
-        CTApplicationNonVisualDrawingProps nv = selectProperty(CTApplicationNonVisualDrawingProps.class, xquery);
-        if (nv == null) return;
-        if(placeholder == null) {
-            if (nv.isSetPh()) nv.unsetPh();
-            _ph = null;
-        } else {
-            nv.addNewPh().setType(STPlaceholderType.Enum.forInt(placeholder.ooxmlId));
-        }
     }
     
+    /**
+     * @see PlaceholderDetails#setPlaceholder(Placeholder)
+     */
+    public void setPlaceholder(final Placeholder placeholder) {
+        getPlaceholderDetails().setPlaceholder(placeholder);
+    }
+
+    /**
+     * @see SimpleShape#getPlaceholderDetails()
+     */
+    public XSLFPlaceholderDetails getPlaceholderDetails() {
+        return new XSLFPlaceholderDetails(this);
+    }
 
     /**
      * As there's no xmlbeans hierarchy, but XSLF works with subclassing, not all
@@ -315,7 +302,7 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
             return true;
         }
 
-        CTPlaceholder ph = getCTPlaceholder();
+        final CTPlaceholder ph = getPlaceholderDetails().getCTPlaceholder(false);
         if (ph == null) {
             return false;
         }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java

@@ -18,6 +18,7 @@ package org.apache.poi.xslf.usermodel;
 
 import static org.apache.poi.POIXMLTypeLoader.DEFAULT_XML_OPTIONS;
 
+import javax.xml.namespace.QName;
 import java.awt.Dimension;
 import java.awt.Graphics2D;
 import java.io.IOException;
@@ -28,8 +29,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-
+import java.util.Optional;
-import javax.xml.namespace.QName;
 
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.POIXMLException;
@@ -56,6 +56,7 @@ import org.apache.xmlbeans.XmlException;
 import org.apache.xmlbeans.XmlObject;
 import org.apache.xmlbeans.XmlOptions;
 import org.apache.xmlbeans.impl.values.XmlAnyTypeImpl;
+import org.openxmlformats.schemas.drawingml.x2006.main.CTColorMapping;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTConnector;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGraphicalObjectFrame;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGroupShape;
@@ -72,6 +73,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
     private XSLFDrawing _drawing;
     private List<XSLFShape> _shapes;
     private CTGroupShape _spTree;
+    private XSLFTheme _theme;
 
     private List<XSLFTextShape>_placeholders;
     private Map<Integer, XSLFSimpleShape> _placeholderByIdMap;
@@ -456,7 +458,36 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
      *  Sheets that support the notion of themes (slides, masters, layouts, etc.) should override this
      *  method and return the corresponding package part.
      */
-    XSLFTheme getTheme(){
+    public XSLFTheme getTheme() {
+        if (_theme != null || !isSupportTheme()) {
+            return _theme;
+        }
+
+        final Optional<XSLFTheme> t =
+                getRelations().stream().filter((p) -> p instanceof XSLFTheme).map((p) -> (XSLFTheme) p).findAny();
+        if (t.isPresent()) {
+            _theme = t.get();
+            final CTColorMapping cmap = getColorMapping();
+            if (cmap != null) {
+                _theme.initColorMap(cmap);
+            }
+        }
+        return _theme;
+    }
+
+
+
+    /**
+     * @return {@code true} if this class supports themes
+     */
+    boolean isSupportTheme() {
+        return false;
+    }
+
+    /**
+     * @return the color mapping for this slide type
+     */
+    CTColorMapping getColorMapping() {
         return null;
     }
 
@@ -488,16 +519,16 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         return shape;
     }
 
-    void initPlaceholders() {
+    private void initPlaceholders() {
         if(_placeholders == null) {
             _placeholders = new ArrayList<>();
             _placeholderByIdMap = new HashMap<>();
             _placeholderByTypeMap = new HashMap<>();
 
-            for(XSLFShape sh : getShapes()){
+            for(final XSLFShape sh : getShapes()){
                 if(sh instanceof XSLFTextShape){
-                    XSLFTextShape sShape = (XSLFTextShape)sh;
+                    final XSLFTextShape sShape = (XSLFTextShape)sh;
-                    CTPlaceholder ph = sShape.getCTPlaceholder();
+                    final CTPlaceholder ph = sShape.getPlaceholderDetails().getCTPlaceholder(false);
                     if(ph != null) {
                         _placeholders.add(sShape);
                         if(ph.isSetIdx()) {
@@ -513,7 +544,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         }
     }
 
-    XSLFSimpleShape getPlaceholderById(int id) {
+    private XSLFSimpleShape getPlaceholderById(int id) {
         initPlaceholders();
         return _placeholderByIdMap.get(id);
     }
@@ -574,7 +605,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
     /**
      * Render this sheet into the supplied graphics object
      *
-     * @param graphics
+     * @param graphics the graphics context to draw to
      */
     @Override
     public void draw(Graphics2D graphics){
@@ -645,4 +676,12 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
     void removePictureRelation(XSLFPictureShape pictureShape) {
         removeRelation(pictureShape.getBlipId());
     }
+
+
+    @Override
+    public XSLFPlaceholderDetails getPlaceholderDetails(Placeholder placeholder) {
+        final XSLFSimpleShape ph = getPlaceholder(placeholder);
+        return (ph == null) ? null : new XSLFPlaceholderDetails(ph);
+    }
+
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSimpleShape.java

@@ -36,7 +36,6 @@ import org.apache.poi.sl.usermodel.LineDecoration.DecorationShape;
 import org.apache.poi.sl.usermodel.LineDecoration.DecorationSize;
 import org.apache.poi.sl.usermodel.PaintStyle;
 import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
-import org.apache.poi.sl.usermodel.Placeholder;
 import org.apache.poi.sl.usermodel.ShapeType;
 import org.apache.poi.sl.usermodel.SimpleShape;
 import org.apache.poi.sl.usermodel.StrokeStyle;
@@ -980,11 +979,6 @@ public abstract class XSLFSimpleShape extends XSLFShape
         return ds; 
     }
 
-    public boolean isPlaceholder() {
-        CTPlaceholder ph = getCTPlaceholder();
-        return ph != null;
-    }
-
     @Override
     public Guide getAdjustValue(String name) {
         XSLFGeometryProperties gp = XSLFPropertiesDelegate.getGeometryDelegate(getShapeProperties());
@@ -1105,11 +1099,6 @@ public abstract class XSLFSimpleShape extends XSLFShape
         }
     }
     
-    @Override
-    public void setPlaceholder(Placeholder placeholder) {
-        super.setPlaceholder(placeholder);
-    }
-    
     @Override
     public XSLFHyperlink getHyperlink() {
         CTNonVisualDrawingProps cNvPr = getCNvPr();

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlide.java

@@ -20,6 +20,8 @@ import static org.apache.poi.POIXMLTypeLoader.DEFAULT_XML_OPTIONS;
 
 import java.awt.Graphics2D;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.openxml4j.opc.PackagePart;
@@ -38,6 +40,7 @@ import org.openxmlformats.schemas.drawingml.x2006.main.CTNonVisualDrawingProps;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTPoint2D;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTPositiveSize2D;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTBackground;
+import org.openxmlformats.schemas.presentationml.x2006.main.CTComment;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTCommonSlideData;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGroupShape;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGroupShapeNonVisual;
@@ -52,6 +55,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
     private final CTSlide _slide;
     private XSLFSlideLayout _layout;
     private XSLFComments _comments;
+    private XSLFCommentAuthors _commentAuthors;
     private XSLFNotes _notes;
 
     /**
@@ -155,20 +159,53 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
         return getSlideLayout().getSlideMaster();
     }
 
-    public XSLFComments getComments() {
+    /**
+     * @return the comments part or {@code null} if there weren't any comments
+     * @since POI 4.0.0
+     */
+    public XSLFComments getCommentsPart() {
         if(_comments == null) {
             for (POIXMLDocumentPart p : getRelations()) {
                 if (p instanceof XSLFComments) {
                     _comments = (XSLFComments)p;
+                    break;
                 }
             }
         }
-        if(_comments == null) {
+
-            // This slide lacks comments
+        return _comments;
-            // Not all have them, sorry...
+    }
+
+    /**
+     * @return the comment authors part or {@code null} if there weren't any comments
+     * @since POI 4.0.0
+     */
+    public XSLFCommentAuthors getCommentAuthorsPart() {
+        if(_commentAuthors == null) {
+            for (POIXMLDocumentPart p : getRelations()) {
+                if (p instanceof XSLFCommentAuthors) {
+                    _commentAuthors = (XSLFCommentAuthors)p;
+                    return _commentAuthors;
+                }
+            }
+        }
+
         return null;
     }
-        return _comments;
+
+
+    @Override
+    public List<XSLFComment> getComments() {
+        final List<XSLFComment> comments = new ArrayList<>();
+        final XSLFComments xComments = getCommentsPart();
+        final XSLFCommentAuthors xAuthors = getCommentAuthorsPart();
+        if (xComments != null) {
+            for (final CTComment xc : xComments.getCTCommentsList().getCmArray()) {
+                comments.add(new XSLFComment(xc, xAuthors));
+            }
+        }
+
+        return comments;
     }
 
     @Override

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideLayout.java

@@ -28,21 +28,15 @@ import org.apache.poi.util.Beta;
 import org.apache.poi.util.Internal;
 import org.apache.xmlbeans.XmlException;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTBackground;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideLayout;
 import org.openxmlformats.schemas.presentationml.x2006.main.SldLayoutDocument;
 
 @Beta
 public class XSLFSlideLayout extends XSLFSheet
 implements MasterSheet<XSLFShape,XSLFTextParagraph> {
-    private CTSlideLayout _layout;
+    private final CTSlideLayout _layout;
     private XSLFSlideMaster _master;
 
-    XSLFSlideLayout() {
-        super();
-        _layout = CTSlideLayout.Factory.newInstance();
-    }
-
     /**
      * @since POI 3.14-Beta1
      */
@@ -111,14 +105,7 @@ implements MasterSheet<XSLFShape,XSLFTextParagraph> {
      */
     @Override
     protected boolean canDraw(XSLFShape shape) {
-        if (shape instanceof XSLFSimpleShape) {
+        return !(shape instanceof XSLFSimpleShape) || !shape.isPlaceholder();
-            XSLFSimpleShape txt = (XSLFSimpleShape) shape;
-            CTPlaceholder ph = txt.getCTPlaceholder();
-            if (ph != null) {
-                return false;
-            }
-        }
-        return true;
     }
 
 

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlideMaster.java

@@ -32,7 +32,6 @@ import org.apache.xmlbeans.XmlException;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTColorMapping;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTTextListStyle;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTBackground;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideMaster;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideMasterTextStyles;
 import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
@@ -43,7 +42,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
 *  Within a slide master slide are contained all elements
 * that describe the objects and their corresponding formatting
 * for within a presentation slide.
-* </p>
 * <p>
 * Within a slide master slide are two main elements.
 * The cSld element specifies the common slide elements such as shapes and
@@ -52,21 +50,12 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
 * within a slide master slide specify other properties for within a presentation slide
 * such as color information, headers and footers, as well as timing and
 * transition information for all corresponding presentation slides.
-* </p>
- *
- * @author Yegor Kozlov
 */
 @Beta
  public class XSLFSlideMaster extends XSLFSheet
  implements MasterSheet<XSLFShape,XSLFTextParagraph> {
 	private CTSlideMaster _slide;
     private Map<String, XSLFSlideLayout> _layouts;
-    private XSLFTheme _theme;
-
-    XSLFSlideMaster() {
-        super();
-        _slide = CTSlideMaster.Factory.newInstance();
-    }
 
     /**
      * @since POI 3.14-Beta1
@@ -142,23 +131,9 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
     }
 
 
-    @Override
-    public XSLFTheme getTheme(){
-        if(_theme == null){
-            for (POIXMLDocumentPart p : getRelations()) {
-                if (p instanceof XSLFTheme){
-                    _theme = (XSLFTheme)p;
-                    CTColorMapping cmap = _slide.getClrMap();
-                    if(cmap != null){
-                        _theme.initColorMap(cmap);
-                    }
-                    break;
-                }
-            }
-        }
-        return _theme;
-    }
 
+
+    @SuppressWarnings(value = "unused")
     protected CTTextListStyle getTextProperties(Placeholder textType) {
         CTTextListStyle props;
         CTSlideMasterTextStyles txStyles = getXmlObject().getTxStyles();
@@ -183,15 +158,8 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
      *
      */
     @Override
-    protected boolean canDraw(XSLFShape shape){
+    protected boolean canDraw(XSLFShape shape) {
-        if(shape instanceof XSLFSimpleShape){
+        return !(shape instanceof XSLFSimpleShape) || !shape.isPlaceholder();
-            XSLFSimpleShape txt = (XSLFSimpleShape)shape;
-            CTPlaceholder ph = txt.getCTPlaceholder();
-            if(ph != null) {
-                return false;
-            }
-        }
-        return true;
     }
 
     @Override
@@ -203,4 +171,14 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
             return null;
         }
     }
+
+    @Override
+    boolean isSupportTheme() {
+        return true;
+    }
+
+    @Override
+    CTColorMapping getColorMapping() {
+        return _slide.getClrMap();
+    }
 }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextParagraph.java

@@ -20,6 +20,9 @@ import java.awt.Color;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 import org.apache.poi.sl.draw.DrawPaint;
 import org.apache.poi.sl.usermodel.AutoNumberingScheme;
@@ -49,6 +52,12 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
     private final List<XSLFTextRun> _runs;
     private final XSLFTextShape _shape;
 
+    @FunctionalInterface
+    private interface Procedure {
+        void accept();
+    }
+
+
     XSLFTextParagraph(CTTextParagraph p, XSLFTextShape shape){
         _p = p;
         _runs = new ArrayList<>();
@@ -79,14 +88,6 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return out.toString();
     }
 
-    String getRenderableText(){
-        StringBuilder out = new StringBuilder();
-        for (XSLFTextRun r : _runs) {
-            out.append(r.getRenderableText());
-        }
-        return out.toString();
-    }
-
     @Internal
     public CTTextParagraph getXmlObject(){
         return _p;
@@ -125,6 +126,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      *
      * @return text run representing this line break ('\n')
      */
+    @SuppressWarnings("WeakerAccess")
     public XSLFTextRun addLineBreak(){
         XSLFLineBreak run = new XSLFLineBreak(_p.addNewBr(), this);
         CTTextCharacterProperties brProps = run.getRPr(true);
@@ -200,6 +202,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
     /**
      * @return the font to be used on bullet characters within a given paragraph
      */
+    @SuppressWarnings("WeakerAccess")
     public String getBulletFont(){
         ParagraphPropertyFetcher<String> fetcher = new ParagraphPropertyFetcher<String>(getIndentLevel()){
             public boolean fetch(CTTextParagraphProperties props){
@@ -214,6 +217,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return fetcher.getValue();
     }
 
+    @SuppressWarnings("WeakerAccess")
     public void setBulletFont(String typeface){
         CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
         CTTextFont font = pr.isSetBuFont() ? pr.getBuFont() : pr.addNewBuFont();
@@ -223,6 +227,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
     /**
      * @return the character to be used in place of the standard bullet point
      */
+    @SuppressWarnings("WeakerAccess")
     public String getBulletCharacter(){
         ParagraphPropertyFetcher<String> fetcher = new ParagraphPropertyFetcher<String>(getIndentLevel()){
             public boolean fetch(CTTextParagraphProperties props){
@@ -237,6 +242,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return fetcher.getValue();
     }
 
+    @SuppressWarnings("WeakerAccess")
     public void setBulletCharacter(String str){
         CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
         CTTextCharBullet c = pr.isSetBuChar() ? pr.getBuChar() : pr.addNewBuChar();
@@ -248,6 +254,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      * @return the color of bullet characters within a given paragraph.
      * A <code>null</code> value means to use the text font color.
      */
+    @SuppressWarnings("WeakerAccess")
     public PaintStyle getBulletFontColor(){
         final XSLFTheme theme = getParentShape().getSheet().getTheme();
         ParagraphPropertyFetcher<Color> fetcher = new ParagraphPropertyFetcher<Color>(getIndentLevel()){
@@ -265,6 +272,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return (col == null) ? null : DrawPaint.createSolidPaint(col);
     }
 
+    @SuppressWarnings("WeakerAccess")
     public void setBulletFontColor(Color color) {
         setBulletFontColor(DrawPaint.createSolidPaint(color));
     }
@@ -275,6 +283,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      *
      * @param color the bullet color
      */
+    @SuppressWarnings("WeakerAccess")
     public void setBulletFontColor(PaintStyle color) {
         if (!(color instanceof SolidPaint)) {
             throw new IllegalArgumentException("Currently XSLF only supports SolidPaint");
@@ -300,6 +309,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      *
      * @return the bullet size
      */
+    @SuppressWarnings("WeakerAccess")
     public Double getBulletFontSize(){
         ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
             public boolean fetch(CTTextParagraphProperties props){
@@ -326,6 +336,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      * If bulletSize < 0, then it specifies the size in points
      * </p>
      */
+    @SuppressWarnings("WeakerAccess")
     public void setBulletFontSize(double bulletSize){
         CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
 
@@ -343,6 +354,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
     /**
      * @return the auto numbering scheme, or null if not defined
      */
+    @SuppressWarnings("WeakerAccess")
     public AutoNumberingScheme getAutoNumberingScheme() {
         ParagraphPropertyFetcher<AutoNumberingScheme> fetcher = new ParagraphPropertyFetcher<AutoNumberingScheme>(getIndentLevel()) {
             public boolean fetch(CTTextParagraphProperties props) {
@@ -363,6 +375,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
     /**
      * @return the auto numbering starting number, or null if not defined
      */
+    @SuppressWarnings("WeakerAccess")
     public Integer getAutoNumberingStartAt() {
         ParagraphPropertyFetcher<Integer> fetcher = new ParagraphPropertyFetcher<Integer>(getIndentLevel()) {
             public boolean fetch(CTTextParagraphProperties props) {
@@ -487,10 +500,11 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return fetcher.getValue();
     }
 
-    public double getTabStop(final int idx){
+    @SuppressWarnings("WeakerAccess")
+    public double getTabStop(final int idx) {
         ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
             public boolean fetch(CTTextParagraphProperties props){
-                if(props.isSetTabLst()){
+                if (props.isSetTabLst()) {
                     CTTextTabStopList tabStops = props.getTabLst();
                     if(idx < tabStops.sizeOfTabArray() ) {
                         CTTextTabStop ts = tabStops.getTabArray(idx);
@@ -506,6 +520,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return fetcher.getValue() == null ? 0. : fetcher.getValue();
     }
 
+    @SuppressWarnings("WeakerAccess")
     public void addTabStop(double value){
         CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
         CTTextTabStopList tabStops = pr.isSetTabLst() ? pr.getTabLst() : pr.addNewTabLst();
@@ -514,45 +529,18 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
 
     @Override
     public void setLineSpacing(Double lineSpacing){
-        if (lineSpacing == null && !_p.isSetPPr()) return;
+        setSpacing(lineSpacing, props -> props::getLnSpc, props -> props::addNewLnSpc, props -> props::unsetLnSpc);
-        CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
-        if(lineSpacing == null) {
-            if (pr.isSetLnSpc()) pr.unsetLnSpc();
-        } else {
-            CTTextSpacing spc = (pr.isSetLnSpc()) ? pr.getLnSpc() : pr.addNewLnSpc();
-            if (lineSpacing >= 0) {
-                (spc.isSetSpcPct() ? spc.getSpcPct() : spc.addNewSpcPct()).setVal((int)(lineSpacing*1000));
-                if (spc.isSetSpcPts()) spc.unsetSpcPts();
-            } else {
-                (spc.isSetSpcPts() ? spc.getSpcPts() : spc.addNewSpcPts()).setVal((int)(-lineSpacing*100));
-                if (spc.isSetSpcPct()) spc.unsetSpcPct();
-            }
-        }
     }
 
     @Override
     public Double getLineSpacing(){
-        ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
+        final Double lnSpc = getSpacing(props -> props::getLnSpc);
-            public boolean fetch(CTTextParagraphProperties props){
-                if(props.isSetLnSpc()){
-                    CTTextSpacing spc = props.getLnSpc();
-
-                    if(spc.isSetSpcPct()) setValue( spc.getSpcPct().getVal()*0.001 );
-                    else if (spc.isSetSpcPts()) setValue( -spc.getSpcPts().getVal()*0.01 );
-                    return true;
-                }
-                return false;
-            }
-        };
-        fetchParagraphProperty(fetcher);
-
-        Double lnSpc = fetcher.getValue();
         if (lnSpc != null && lnSpc > 0) {
             // check if the percentage value is scaled
-            CTTextNormalAutofit normAutofit = getParentShape().getTextBodyPr().getNormAutofit();
+            final CTTextNormalAutofit normAutofit = getParentShape().getTextBodyPr().getNormAutofit();
-            if(normAutofit != null) {
+            if (normAutofit != null) {
-                double scale = 1 - (double)normAutofit.getLnSpcReduction() / 100000;
+                final double scale = 1 - (double)normAutofit.getLnSpcReduction() / 100000;
-                lnSpc *= scale;
+                return lnSpc * scale;
             }
         }
         
@@ -561,84 +549,82 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
 
     @Override
     public void setSpaceBefore(Double spaceBefore){
-        if (spaceBefore == null && !_p.isSetPPr()) {
+        setSpacing(spaceBefore, props -> props::getSpcBef, props -> props::addNewSpcBef, props -> props::unsetSpcBef);
-            return;
-        }
-
-        // unset the space before on null input
-        if (spaceBefore == null) {
-            if(_p.getPPr().isSetSpcBef()) {
-                _p.getPPr().unsetSpcBef();
-            }
-            return;
-        }
-
-        CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
-        CTTextSpacing spc = CTTextSpacing.Factory.newInstance();
-
-        if(spaceBefore >= 0) {
-            spc.addNewSpcPct().setVal((int)(spaceBefore*1000));
-        } else {
-            spc.addNewSpcPts().setVal((int)(-spaceBefore*100));
-        }
-        pr.setSpcBef(spc);
     }
 
     @Override
     public Double getSpaceBefore(){
-        ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
+        return getSpacing(props -> props::getSpcBef);
-            public boolean fetch(CTTextParagraphProperties props){
-                if(props.isSetSpcBef()){
-                    CTTextSpacing spc = props.getSpcBef();
-
-                    if(spc.isSetSpcPct()) setValue( spc.getSpcPct().getVal()*0.001 );
-                    else if (spc.isSetSpcPts()) setValue( -spc.getSpcPts().getVal()*0.01 );
-                    return true;
-                }
-                return false;
-            }
-        };
-        fetchParagraphProperty(fetcher);
-
-        return fetcher.getValue();
     }
 
     @Override
     public void setSpaceAfter(Double spaceAfter){
-        if (spaceAfter == null && !_p.isSetPPr()) {
+        setSpacing(spaceAfter, props -> props::getSpcAft, props -> props::addNewSpcAft, props -> props::unsetSpcAft);
-            return;
-        }
-
-        // unset the space before on null input
-        if (spaceAfter == null) {
-            if(_p.getPPr().isSetSpcAft()) {
-                _p.getPPr().unsetSpcAft();
-            }
-            return;
-        }
-
-        CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
-        CTTextSpacing spc = CTTextSpacing.Factory.newInstance();
-
-        if(spaceAfter >= 0) {
-            spc.addNewSpcPct().setVal((int)(spaceAfter*1000));
-        } else {
-            spc.addNewSpcPts().setVal((int)(-spaceAfter*100));
-        }
-        pr.setSpcAft(spc);
     }
 
     @Override
-    public Double getSpaceAfter(){
+    public Double getSpaceAfter() {
-        ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
+        return getSpacing(props -> props::getSpcAft);
-            public boolean fetch(CTTextParagraphProperties props){
+    }
-                if(props.isSetSpcAft()){
-                    CTTextSpacing spc = props.getSpcAft();
 
-                    if(spc.isSetSpcPct()) setValue( spc.getSpcPct().getVal()*0.001 );
+    private void setSpacing(final Double space,
-                    else if (spc.isSetSpcPts()) setValue( -spc.getSpcPts().getVal()*0.01 );
+        final Function<CTTextParagraphProperties,Supplier<CTTextSpacing>> getSpc,
+        final Function<CTTextParagraphProperties,Supplier<CTTextSpacing>> addSpc,
+        final Function<CTTextParagraphProperties,Procedure> unsetSpc
+    ) {
+        final CTTextParagraphProperties pPr = (space == null || _p.isSetPPr()) ?  _p.getPPr() : _p.addNewPPr();
+        if (pPr == null) {
+            return;
+        }
+
+        CTTextSpacing spc = getSpc.apply(pPr).get();
+
+        if (space == null) {
+            if (spc != null) {
+                // unset the space before on null input
+                unsetSpc.apply(pPr).accept();
+            }
+            return;
+        }
+
+        if (spc == null) {
+            spc = addSpc.apply(pPr).get();
+        }
+
+        if (space >= 0) {
+            if (spc.isSetSpcPts()) {
+                spc.unsetSpcPts();
+            }
+            final CTTextSpacingPercent pct = spc.isSetSpcPct() ? spc.getSpcPct() : spc.addNewSpcPct();
+            pct.setVal((int)(space*1000));
+        } else {
+            if (spc.isSetSpcPct()) {
+                spc.unsetSpcPct();
+            }
+            final CTTextSpacingPoint pts = spc.isSetSpcPts() ? spc.getSpcPts() : spc.addNewSpcPts();
+            pts.setVal((int)(-space*100));
+        }
+    }
+
+    private Double getSpacing(final Function<CTTextParagraphProperties,Supplier<CTTextSpacing>> getSpc) {
+        final ParagraphPropertyFetcher<Double> fetcher = new ParagraphPropertyFetcher<Double>(getIndentLevel()){
+            public boolean fetch(final CTTextParagraphProperties props){
+                final CTTextSpacing spc = getSpc.apply(props).get();
+
+                if (spc == null) {
+                    return false;
+                }
+
+                if (spc.isSetSpcPct()) {
+                    setValue( spc.getSpcPct().getVal()*0.001 );
                     return true;
                 }
+
+                if (spc.isSetSpcPts()) {
+                    setValue( -spc.getSpcPts().getVal()*0.01 );
+                    return true;
+                }
+
                 return false;
             }
         };
@@ -713,6 +699,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      * @param startAt the number that will start number for a given sequence of automatically
     numbered bullets (1-based).
      */
+    @SuppressWarnings("WeakerAccess")
     public void setBulletAutoNumber(AutoNumberingScheme scheme, int startAt) {
         if(startAt < 1) throw new IllegalArgumentException("Start Number must be greater or equal that 1") ;
         CTTextParagraphProperties pr = _p.isSetPPr() ? _p.getPPr() : _p.addNewPPr();
@@ -732,7 +719,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
      * there are no master slides or the master slides do not contain a text paragraph
      */
     /* package */ CTTextParagraphProperties getDefaultMasterStyle(){
-        CTPlaceholder ph = _shape.getCTPlaceholder();
+        CTPlaceholder ph = _shape.getPlaceholderDetails().getCTPlaceholder(false);
         String defaultStyleSelector;  
         switch(ph == null ? -1 : ph.getType().intValue()) {
             case STPlaceholderType.INT_TITLE:
@@ -780,37 +767,46 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
         return null;
     }
 
-    private <T> boolean fetchParagraphProperty(ParagraphPropertyFetcher<T> visitor){
+    private void fetchParagraphProperty(final ParagraphPropertyFetcher<?> visitor){
-        boolean ok = false;
         final XSLFTextShape shape = getParentShape();
         final XSLFSheet sheet = shape.getSheet();
         
         if (!(sheet instanceof XSLFSlideMaster)) {
-            if(_p.isSetPPr()) ok = visitor.fetch(_p.getPPr());
+            if (_p.isSetPPr() && visitor.fetch(_p.getPPr())) {
-            if (ok) return true;
+                return;
+            }
     
-            ok = shape.fetchShapeProperty(visitor);
+            if (shape.fetchShapeProperty(visitor)) {
-            if (ok) return true;
+                return;
+            }
 
+            if (fetchThemeProperty(visitor)) {
+                return;
+            }
+        }
+
+        fetchMasterProperty(visitor);
+    }
+
+    boolean fetchMasterProperty(final ParagraphPropertyFetcher<?> visitor) {
+        // defaults for placeholders are defined in the slide master
+        final CTTextParagraphProperties defaultProps = getDefaultMasterStyle();
+        // TODO: determine master shape
+        return defaultProps != null && visitor.fetch(defaultProps);
+    }
+
+    boolean fetchThemeProperty(final ParagraphPropertyFetcher<?> visitor) {
+        final XSLFTextShape shape = getParentShape();
+
+        if (shape.isPlaceholder()) {
+            return false;
+        }
 
-            CTPlaceholder ph = shape.getCTPlaceholder();
-            if(ph == null){
         // if it is a plain text box then take defaults from presentation.xml
         @SuppressWarnings("resource")
-                XMLSlideShow ppt = sheet.getSlideShow();
+        final XMLSlideShow ppt = shape.getSheet().getSlideShow();
-                CTTextParagraphProperties themeProps = ppt.getDefaultParagraphStyle(getIndentLevel());
+        final CTTextParagraphProperties themeProps = ppt.getDefaultParagraphStyle(getIndentLevel());
-                if (themeProps != null) ok = visitor.fetch(themeProps);
+        return themeProps != null && visitor.fetch(themeProps);
-            }
-            if (ok) return true;
-        }
-
-        // defaults for placeholders are defined in the slide master
-        CTTextParagraphProperties defaultProps = getDefaultMasterStyle();
-        // TODO: determine master shape
-        if(defaultProps != null) ok = visitor.fetch(defaultProps);
-        if (ok) return true;
-
-        return false;
     }
 
     void copy(XSLFTextParagraph other){
@@ -873,40 +869,40 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
                     setBulletFontColor(buColor);
                 }
                 Double buSize = other.getBulletFontSize();
-                if(!doubleEquals(buSize, getBulletFontSize())){
+                if(doubleNotEquals(buSize, getBulletFontSize())){
                     setBulletFontSize(buSize);
                 }
             }
         }
 
         Double leftMargin = other.getLeftMargin();
-        if (!doubleEquals(leftMargin, getLeftMargin())){
+        if (doubleNotEquals(leftMargin, getLeftMargin())){
             setLeftMargin(leftMargin);
         }
 
         Double indent = other.getIndent();
-        if (!doubleEquals(indent, getIndent())) {
+        if (doubleNotEquals(indent, getIndent())) {
             setIndent(indent);
         }
 
         Double spaceAfter = other.getSpaceAfter();
-        if (!doubleEquals(spaceAfter, getSpaceAfter())) {
+        if (doubleNotEquals(spaceAfter, getSpaceAfter())) {
             setSpaceAfter(spaceAfter);
         }
         
         Double spaceBefore = other.getSpaceBefore();
-        if (!doubleEquals(spaceBefore, getSpaceBefore())) {
+        if (doubleNotEquals(spaceBefore, getSpaceBefore())) {
             setSpaceBefore(spaceBefore);
         }
         
         Double lineSpacing = other.getLineSpacing();
-        if (!doubleEquals(lineSpacing, getLineSpacing())) {
+        if (doubleNotEquals(lineSpacing, getLineSpacing())) {
             setLineSpacing(lineSpacing);
         }
     }
 
-    private static boolean doubleEquals(Double d1, Double d2) {
+    private static boolean doubleNotEquals(Double d1, Double d2) {
-        return (d1 == d2 || (d1 != null && d1.equals(d2)));
+        return !Objects.equals(d1, d2);
     }
     
     @Override
@@ -1072,7 +1068,7 @@ public class XSLFTextParagraph implements TextParagraph<XSLFShape,XSLFTextParagr
 
     @Override
     public boolean isHeaderOrFooter() {
-        CTPlaceholder ph = _shape.getCTPlaceholder();
+        CTPlaceholder ph = _shape.getPlaceholderDetails().getCTPlaceholder(false);
         int phId = (ph == null ? -1 : ph.getType().intValue());
         switch (phId) {
             case STPlaceholderType.INT_SLD_NUM:

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java

@@ -17,7 +17,6 @@
 package org.apache.poi.xslf.usermodel;
 
 import java.awt.Color;
-import java.util.Locale;
 
 import org.apache.poi.common.usermodel.fonts.FontCharset;
 import org.apache.poi.common.usermodel.fonts.FontFamily;
@@ -31,7 +30,6 @@ import org.apache.poi.sl.usermodel.PaintStyle;
 import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
 import org.apache.poi.sl.usermodel.TextRun;
 import org.apache.poi.util.Beta;
-import org.apache.poi.util.LocaleUtil;
 import org.apache.poi.xslf.model.CharacterPropertyFetcher;
 import org.apache.poi.xslf.usermodel.XSLFPropertiesDelegate.XSLFFillProperties;
 import org.apache.xmlbeans.XmlObject;
@@ -47,10 +45,8 @@ import org.openxmlformats.schemas.drawingml.x2006.main.CTTextField;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTTextFont;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTTextLineBreak;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTTextNormalAutofit;
-import org.openxmlformats.schemas.drawingml.x2006.main.CTTextParagraphProperties;
 import org.openxmlformats.schemas.drawingml.x2006.main.STTextStrikeType;
 import org.openxmlformats.schemas.drawingml.x2006.main.STTextUnderlineType;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
 
 /**
  * Represents a run of text within the containing text body. The run element is the
@@ -83,36 +79,6 @@ public class XSLFTextRun implements TextRun {
         return ((CTRegularTextRun)_r).getT();
     }
 
-    String getRenderableText(){
-        if (_r instanceof CTTextField) {
-            CTTextField tf = (CTTextField)_r;
-            XSLFSheet sheet = _p.getParentShape().getSheet();
-            if ("slidenum".equals(tf.getType()) && sheet instanceof XSLFSlide) {
-                return Integer.toString(((XSLFSlide)sheet).getSlideNumber());
-            }
-            return tf.getT();
-        } else if (_r instanceof CTTextLineBreak) {
-            return "\n";
-        }
-
-        return getRenderableText(((CTRegularTextRun)_r).getT());
-    }
-
-    /* package */ String getRenderableText(final String txt){
-        // TODO: finish support for tabs
-        final String txtSpace = txt.replace("\t", "  ");
-        final Locale loc = LocaleUtil.getUserLocale();
-
-        switch (getTextCap()) {
-            case ALL:
-                return txtSpace.toUpperCase(loc);
-            case SMALL:
-                return txtSpace.toLowerCase(loc);
-            default:
-                return txtSpace;
-        }
-    }
-
     @Override
     public void setText(String text){
         if (_r instanceof CTTextField) {
@@ -230,6 +196,7 @@ public class XSLFTextRun implements TextRun {
      * @return the spacing between characters within a text run,
      * If this attribute is omitted than a value of 0 or no adjustment is assumed.
      */
+    @SuppressWarnings("WeakerAccess")
     public double getCharacterSpacing(){
 
         CharacterPropertyFetcher<Double> fetcher = new CharacterPropertyFetcher<Double>(_p.getIndentLevel()){
@@ -255,6 +222,7 @@ public class XSLFTextRun implements TextRun {
      *
      * @param spc  character spacing in points.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setCharacterSpacing(double spc){
         CTTextCharacterProperties rPr = getRPr(true);
         if(spc == 0.0) {
@@ -357,9 +325,8 @@ public class XSLFTextRun implements TextRun {
      *     The size is specified using a percentage.
      *     Positive values indicate superscript, negative values indicate subscript.
      *  </p>
-     *
-     * @param baselineOffset
      */
+    @SuppressWarnings("WeakerAccess")
     public void setBaselineOffset(double baselineOffset){
        getRPr(true).setBaseline((int) baselineOffset * 1000);
     }
@@ -370,6 +337,7 @@ public class XSLFTextRun implements TextRun {
      *
      * @see #setBaselineOffset(double)
      */
+    @SuppressWarnings("WeakerAccess")
     public void setSuperscript(boolean flag){
         setBaselineOffset(flag ? 30. : 0.);
     }
@@ -380,6 +348,7 @@ public class XSLFTextRun implements TextRun {
      *
      * @see #setBaselineOffset(double)
      */
+    @SuppressWarnings("WeakerAccess")
     public void setSubscript(boolean flag){
         setBaselineOffset(flag ? -25.0 : 0.);
     }
@@ -544,38 +513,23 @@ public class XSLFTextRun implements TextRun {
         return new XSLFHyperlink(hl, _p.getParentShape().getSheet());
     }
 
-    private boolean fetchCharacterProperty(CharacterPropertyFetcher<?> fetcher){
+    private void fetchCharacterProperty(final CharacterPropertyFetcher<?> visitor){
         XSLFTextShape shape = _p.getParentShape();
-        XSLFSheet sheet = shape.getSheet();
 
         CTTextCharacterProperties rPr = getRPr(false);
-        if (rPr != null && fetcher.fetch(rPr)) {
+        if (rPr != null && visitor.fetch(rPr)) {
-            return true;
+            return;
         }
 
-        if (shape.fetchShapeProperty(fetcher)) {
+        if (shape.fetchShapeProperty(visitor)) {
-            return true;
+            return;
         }
 
-        CTPlaceholder ph = shape.getCTPlaceholder();
+        if (_p.fetchThemeProperty(visitor)) {
-        if (ph == null){
+            return;
-            // if it is a plain text box then take defaults from presentation.xml
-            @SuppressWarnings("resource")
-            XMLSlideShow ppt = sheet.getSlideShow();
-            // TODO: determine master shape
-            CTTextParagraphProperties themeProps = ppt.getDefaultParagraphStyle(_p.getIndentLevel());
-            if (themeProps != null && fetcher.fetch(themeProps)) {
-                return true;
-            }
         }
 
-        // TODO: determine master shape
+        _p.fetchMasterProperty(visitor);
-        CTTextParagraphProperties defaultProps =  _p.getDefaultMasterStyle();
-        if(defaultProps != null && fetcher.fetch(defaultProps)) {
-            return true;
-        }
-
-        return false;
     }
 
     void copy(XSLFTextRun r){
@@ -630,14 +584,14 @@ public class XSLFTextRun implements TextRun {
     }
 
 
-    private class XSLFFontInfo implements FontInfo {
+    private final class XSLFFontInfo implements FontInfo {
         private final FontGroup fontGroup;
 
         private XSLFFontInfo(FontGroup fontGroup) {
             this.fontGroup = (fontGroup != null) ? fontGroup : FontGroup.getFontGroupFirst(getRawText());
         }
 
-        public void copyFrom(FontInfo fontInfo) {
+        void copyFrom(FontInfo fontInfo) {
             CTTextFont tf = getXmlObject(true);
             setTypeface(fontInfo.getTypeface());
             setCharset(fontInfo.getCharset());

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextShape.java

@@ -590,13 +590,7 @@ public abstract class XSLFTextShape extends XSLFSimpleShape
     }
 
     public Placeholder getTextType(){
-        CTPlaceholder ph = getCTPlaceholder();
+        return getPlaceholder();
-        if (ph == null) {
-            return null;
-        }
-
-        int val = ph.getType().intValue();
-        return Placeholder.lookupOoxml(val);
     }
 
     @Override

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXMLSlideShow.java

@@ -19,6 +19,7 @@ package org.apache.poi.xslf.usermodel;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -51,7 +52,7 @@ public class TestXMLSlideShow extends BaseTestSlideShow {
    }
    
    @After
-   public void tearDown() throws IOException {
+   public void tearDown() {
        pack.revert();
    }
 
@@ -149,7 +150,7 @@ public class TestXMLSlideShow extends BaseTestSlideShow {
       assertEquals(null, xml.getCommentAuthors());
       
       for (XSLFSlide slide : xml.getSlides()) {
-         assertEquals(null, slide.getComments());
+         assertTrue(slide.getComments().isEmpty());
       }
       
       // Try another with comments
@@ -166,17 +167,18 @@ public class TestXMLSlideShow extends BaseTestSlideShow {
          i++;
          
          if(i == 0) {
-            assertNotNull(slide.getComments());
+            assertNotNull(slide.getCommentsPart());
-            assertEquals(1, slide.getComments().getNumberOfComments());
+            assertEquals(1, slide.getCommentsPart().getNumberOfComments());
-            assertEquals("testdoc", slide.getComments().getCommentAt(0).getText());
+            assertEquals("testdoc", slide.getCommentsPart().getCommentAt(0).getText());
-            assertEquals(0, slide.getComments().getCommentAt(0).getAuthorId());
+            assertEquals(0, slide.getCommentsPart().getCommentAt(0).getAuthorId());
          } else if (i == 1) {
             assertNotNull(slide.getComments());
-            assertEquals(1, slide.getComments().getNumberOfComments());
+            assertEquals(1, slide.getCommentsPart().getNumberOfComments());
-            assertEquals("test phrase", slide.getComments().getCommentAt(0).getText());
+            assertEquals("test phrase", slide.getCommentsPart().getCommentAt(0).getText());
-            assertEquals(0, slide.getComments().getCommentAt(0).getAuthorId());
+            assertEquals(0, slide.getCommentsPart().getCommentAt(0).getAuthorId());
          } else {
-            assertEquals(null, slide.getComments());
+            assertNull(slide.getCommentsPart());
+            assertTrue(slide.getComments().isEmpty());
          }
       }
       
@@ -188,7 +190,7 @@ public class TestXMLSlideShow extends BaseTestSlideShow {
        return reopen((XMLSlideShow)show);
    }
 
-   public static XMLSlideShow reopen(XMLSlideShow show) {
+   private static XMLSlideShow reopen(XMLSlideShow show) {
        try {
            BufAccessBAOS bos = new BufAccessBAOS();
            show.write(bos);
@@ -200,7 +202,7 @@ public class TestXMLSlideShow extends BaseTestSlideShow {
    }
 
    private static class BufAccessBAOS extends ByteArrayOutputStream {
-       public byte[] getBuf() {
+       byte[] getBuf() {
            return buf;
        }
    }

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTextRun.java

@@ -28,6 +28,7 @@ import static org.mockito.Mockito.when;
 import java.awt.Color;
 import java.io.IOException;
 
+import org.apache.poi.sl.draw.DrawTextParagraph;
 import org.junit.Test;
 
 /**
@@ -65,17 +66,17 @@ public class TestXSLFTextRun {
         r.setFontSize(13.0);
         assertEquals(13.0, r.getFontSize(), 0);
 
-        assertEquals(false, r.isSuperscript());
+        assertFalse(r.isSuperscript());
         r.setSuperscript(true);
-        assertEquals(true, r.isSuperscript());
+        assertTrue(r.isSuperscript());
         r.setSuperscript(false);
-        assertEquals(false, r.isSuperscript());
+        assertFalse(r.isSuperscript());
 
-        assertEquals(false, r.isSubscript());
+        assertFalse(r.isSubscript());
         r.setSubscript(true);
-        assertEquals(true, r.isSubscript());
+        assertTrue(r.isSubscript());
         r.setSubscript(false);
-        assertEquals(false, r.isSubscript());
+        assertFalse(r.isSubscript());
         
         ppt.close();
     }
@@ -94,8 +95,11 @@ public class TestXSLFTextRun {
         try (XMLSlideShow ppt = new XMLSlideShow()) {
             XSLFSlide slide = ppt.createSlide();
             XSLFTextShape sh = slide.createAutoShape();
-            XSLFTextRun r = sh.addNewTextParagraph().addNewTextRun();
+            XSLFTextParagraph p = sh.addNewTextParagraph();
-            assertEquals(unicodeSurrogates, r.getRenderableText(unicodeSurrogates));
+            XSLFTextRun r = p.addNewTextRun();
+            r.setText(unicodeSurrogates);
+
+            assertEquals(unicodeSurrogates, new DrawTextParagraph(p).getRenderableText(r));
         }
     }
 

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTextShape.java

@@ -83,7 +83,7 @@ public class TestXSLFTextShape {
         assertEquals("Title Slide",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.CTR_TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -113,7 +113,7 @@ public class TestXSLFTextShape {
         assertTrue(sameColor(Color.black, r1.getFontColor()));
 
         XSLFTextShape shape2 = (XSLFTextShape)shapes.get(1);
-        CTPlaceholder ph2 = shape2.getCTPlaceholder();
+        CTPlaceholder ph2 = shape2.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.SUB_TITLE, ph2.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape2).getXfrm());
@@ -149,7 +149,7 @@ public class TestXSLFTextShape {
         assertEquals("Title and Content",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -182,7 +182,7 @@ public class TestXSLFTextShape {
         assertTrue(sameColor(Color.black, r1.getFontColor()));
 
         XSLFTextShape shape2 = (XSLFTextShape)shapes.get(1);
-        CTPlaceholder ph2 = shape2.getCTPlaceholder();
+        CTPlaceholder ph2 = shape2.getPlaceholderDetails().getCTPlaceholder(false);
         assertFalse(ph2.isSetType()); // <p:ph idx="1"/>
         assertTrue(ph2.isSetIdx());
         assertEquals(1, ph2.getIdx());
@@ -262,7 +262,7 @@ public class TestXSLFTextShape {
         assertEquals("Section Header",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -296,7 +296,7 @@ public class TestXSLFTextShape {
         assertFalse(r1.isUnderlined());
 
         XSLFTextShape shape2 = (XSLFTextShape)shapes.get(1);
-        CTPlaceholder ph2 = shape2.getCTPlaceholder();
+        CTPlaceholder ph2 = shape2.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.BODY, ph2.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape2).getXfrm());
@@ -333,7 +333,7 @@ public class TestXSLFTextShape {
         assertEquals("Two Content",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -367,7 +367,7 @@ public class TestXSLFTextShape {
         assertTrue(sameColor(Color.black, r1.getFontColor()));
 
         XSLFTextShape shape2 = (XSLFTextShape)shapes.get(1);
-        CTPlaceholder ph2 = shape2.getCTPlaceholder();
+        CTPlaceholder ph2 = shape2.getPlaceholderDetails().getCTPlaceholder(false);
         assertFalse(ph2.isSetType());
         assertTrue(ph2.isSetIdx());
         assertEquals(1, ph2.getIdx());  //<p:ph sz="half" idx="1"/>
@@ -448,7 +448,7 @@ public class TestXSLFTextShape {
         assertEquals("Blank",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -516,7 +516,7 @@ public class TestXSLFTextShape {
         assertEquals("Content with Caption",layout.getName());
 
         XSLFTextShape shape1 = (XSLFTextShape)shapes.get(0);
-        CTPlaceholder ph1 = shape1.getCTPlaceholder();
+        CTPlaceholder ph1 = shape1.getPlaceholderDetails().getCTPlaceholder(false);
         assertEquals(STPlaceholderType.TITLE, ph1.getType());
         // anchor is not defined in the shape
         assertNull(getSpPr(shape1).getXfrm());
@@ -549,7 +549,7 @@ public class TestXSLFTextShape {
         assertTrue(r1.isBold());
 
         XSLFTextShape shape2 = (XSLFTextShape)shapes.get(1);
-        CTPlaceholder ph2 = shape2.getCTPlaceholder();
+        CTPlaceholder ph2 = shape2.getPlaceholderDetails().getCTPlaceholder(false);
         assertFalse(ph2.isSetType());
         assertTrue(ph2.isSetIdx());
         assertEquals(1, ph2.getIdx());

src/scratchpad/src/org/apache/poi/hslf/extractor/PowerPointExtractor.java

@@ -20,47 +20,36 @@ package org.apache.poi.hslf.extractor;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.poi.POIOLE2TextExtractor;
-import org.apache.poi.hslf.model.Comment;
+import org.apache.poi.hslf.usermodel.HSLFObjectShape;
-import org.apache.poi.hslf.model.HSLFMetroShape;
-import org.apache.poi.hslf.model.HeadersFooters;
-import org.apache.poi.hslf.usermodel.HSLFMasterSheet;
-import org.apache.poi.hslf.usermodel.HSLFNotes;
 import org.apache.poi.hslf.usermodel.HSLFShape;
-import org.apache.poi.hslf.usermodel.HSLFSlide;
-import org.apache.poi.hslf.usermodel.HSLFSlideMaster;
 import org.apache.poi.hslf.usermodel.HSLFSlideShow;
 import org.apache.poi.hslf.usermodel.HSLFSlideShowImpl;
-import org.apache.poi.hslf.usermodel.HSLFTable;
-import org.apache.poi.hslf.usermodel.HSLFTableCell;
 import org.apache.poi.hslf.usermodel.HSLFTextParagraph;
-import org.apache.poi.hslf.usermodel.HSLFTextShape;
+import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey;
-import org.apache.poi.hslf.usermodel.HSLFObjectShape;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
-import org.apache.poi.util.POILogFactory;
+import org.apache.poi.sl.extractor.SlideShowExtractor;
-import org.apache.poi.util.POILogger;
+import org.apache.poi.sl.usermodel.SlideShowFactory;
 
 /**
  * This class can be used to extract text from a PowerPoint file. Can optionally
  * also get the notes from one.
+ *
+ * @deprecated in POI 4.0.0, use {@link SlideShowExtractor} instead
  */
+@SuppressWarnings("WeakerAccess")
+@Deprecated
 public final class PowerPointExtractor extends POIOLE2TextExtractor {
-   private static final POILogger LOG = POILogFactory.getLogger(PowerPointExtractor.class);
+   	private final SlideShowExtractor<HSLFShape,HSLFTextParagraph> delegate;
 
-   private final HSLFSlideShow _show;
+	private boolean slidesByDefault = true;
-   private final List<HSLFSlide> _slides;
+	private boolean notesByDefault;
-
+	private boolean commentsByDefault;
-   private boolean _slidesByDefault = true;
+	private boolean masterByDefault;
-   private boolean _notesByDefault;
-   private boolean _commentsByDefault;
-   private boolean _masterByDefault;
 
 	/**
 	 * Basic extractor. Returns all the text, and optionally all the notes
@@ -92,13 +81,19 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 		ppe.close();
 	}
 
+	public PowerPointExtractor(final HSLFSlideShow slideShow) {
+		super(slideShow.getSlideShowImpl());
+		setFilesystem(slideShow);
+		delegate = new SlideShowExtractor<>(slideShow);
+	}
+
 	/**
 	 * Creates a PowerPointExtractor, from a file
 	 *
 	 * @param fileName The name of the file to extract from
 	 */
 	public PowerPointExtractor(String fileName) throws IOException {
-		this(new NPOIFSFileSystem(new File(fileName)));
+		this((HSLFSlideShow)SlideShowFactory.create(new File(fileName), Biff8EncryptionKey.getCurrentUserPassword(), true));
 	}
 
 	/**
@@ -107,7 +102,7 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 	 * @param iStream The input stream containing the PowerPoint document
 	 */
 	public PowerPointExtractor(InputStream iStream) throws IOException {
-		this(new POIFSFileSystem(iStream));
+		this((HSLFSlideShow)SlideShowFactory.create(iStream, Biff8EncryptionKey.getCurrentUserPassword()));
 	}
 
 	/**
@@ -116,7 +111,7 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 	 * @param fs the POIFSFileSystem containing the PowerPoint document
 	 */
 	public PowerPointExtractor(POIFSFileSystem fs) throws IOException {
-		this(fs.getRoot());
+		this((HSLFSlideShow)SlideShowFactory.create(fs, Biff8EncryptionKey.getCurrentUserPassword()));
 	}
 
    /**
@@ -125,8 +120,7 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
     * @param fs the NPOIFSFileSystem containing the PowerPoint document
     */
    public PowerPointExtractor(NPOIFSFileSystem fs) throws IOException {
-      this(fs.getRoot());
+	   this((HSLFSlideShow)SlideShowFactory.create(fs, Biff8EncryptionKey.getCurrentUserPassword()));
-      setFilesystem(fs);
    }
 
    /**
@@ -136,7 +130,7 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
     * @param dir the POIFS Directory containing the PowerPoint document
     */
    public PowerPointExtractor(DirectoryNode dir) throws IOException {
-      this(new HSLFSlideShowImpl(dir));
+      this(new HSLFSlideShow(dir));
    }
 
 	/**
@@ -145,37 +139,39 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 	 * @param ss the HSLFSlideShow to extract text from
 	 */
 	public PowerPointExtractor(HSLFSlideShowImpl ss) {
-		super(ss);
+		this(new HSLFSlideShow(ss));
-		_show = new HSLFSlideShow(ss);
-		_slides = _show.getSlides();
 	}
 
 	/**
 	 * Should a call to getText() return slide text? Default is yes
 	 */
-	public void setSlidesByDefault(boolean slidesByDefault) {
+	public void setSlidesByDefault(final boolean slidesByDefault) {
-		this._slidesByDefault = slidesByDefault;
+		this.slidesByDefault = slidesByDefault;
+		delegate.setSlidesByDefault(slidesByDefault);
 	}
 
 	/**
 	 * Should a call to getText() return notes text? Default is no
 	 */
-	public void setNotesByDefault(boolean notesByDefault) {
+	public void setNotesByDefault(final boolean notesByDefault) {
-		this._notesByDefault = notesByDefault;
+		this.notesByDefault = notesByDefault;
+		delegate.setNotesByDefault(notesByDefault);
 	}
 
 	/**
 	 * Should a call to getText() return comments text? Default is no
 	 */
-	public void setCommentsByDefault(boolean commentsByDefault) {
+	public void setCommentsByDefault(final boolean commentsByDefault) {
-		this._commentsByDefault = commentsByDefault;
+		this.commentsByDefault = commentsByDefault;
+		delegate.setCommentsByDefault(commentsByDefault);
 	}
 
     /**
      * Should a call to getText() return text from master? Default is no
      */
-    public void setMasterByDefault(boolean masterByDefault) {
+    public void setMasterByDefault(final boolean masterByDefault) {
-        this._masterByDefault = masterByDefault;
+    	this.masterByDefault = masterByDefault;
+    	delegate.setMasterByDefault(masterByDefault);
     }
 
 	/**
@@ -184,28 +180,7 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 	 */
 	@Override
     public String getText() {
-		return getText(_slidesByDefault, _notesByDefault, _commentsByDefault, _masterByDefault);
+		return delegate.getText();
-	}
-
-	/**
-	 * Fetches all the notes text from the slideshow, but not the slide text
-	 */
-	public String getNotes() {
-		return getText(false, true);
-	}
-
-	public List<HSLFObjectShape> getOLEShapes() {
-		List<HSLFObjectShape> list = new ArrayList<>();
-
-		for (HSLFSlide slide : _slides) {
-			for (HSLFShape shape : slide.getShapes()) {
-				if (shape instanceof HSLFObjectShape) {
-					list.add((HSLFObjectShape) shape);
-				}
-			}
-		}
-
-		return list;
 	}
 
 	/**
@@ -217,159 +192,33 @@ public final class PowerPointExtractor extends POIOLE2TextExtractor {
 	 * @param getNoteText fetch note text
 	 */
 	public String getText(boolean getSlideText, boolean getNoteText) {
-		return getText(getSlideText, getNoteText, _commentsByDefault, _masterByDefault);
+		return getText(getSlideText,getNoteText,commentsByDefault,masterByDefault);
 	}
 
 	public String getText(boolean getSlideText, boolean getNoteText, boolean getCommentText, boolean getMasterText) {
-		StringBuffer ret = new StringBuffer();
+		delegate.setSlidesByDefault(getSlideText);
-
+		delegate.setNotesByDefault(getNoteText);
-		if (getSlideText) {
+		delegate.setCommentsByDefault(getCommentText);
-            if (getMasterText) {
+		delegate.setMasterByDefault(getMasterText);
-                for (HSLFSlideMaster master : _show.getSlideMasters()) {
+		try {
-                    for(HSLFShape sh : master.getShapes()){
+			return delegate.getText();
-                        if(sh instanceof HSLFTextShape){
+		} finally {
-                            HSLFTextShape hsh = (HSLFTextShape)sh;
+			delegate.setSlidesByDefault(slidesByDefault);
-                            final String text = hsh.getText();
+			delegate.setNotesByDefault(notesByDefault);
-                            if (text == null || text.isEmpty() || "*".equals(text)) {
+			delegate.setCommentsByDefault(commentsByDefault);
-                                continue;
+			delegate.setMasterByDefault(masterByDefault);
-                            }
-                            
-                            if (HSLFMasterSheet.isPlaceholder(sh)) {
-                                // check for metro shape of complex placeholder
-                                boolean isMetro = new HSLFMetroShape<HSLFShape>(sh).hasMetroBlob();
-                                
-                                if (!isMetro) {
-                                    // don't bother about boiler plate text on master sheets
-                                    LOG.log(POILogger.INFO, "Ignoring boiler plate (placeholder) text on slide master:", text);
-                                    continue;
 		}
 	}
 
-                            ret.append(text);
+	/**
-                            if (!text.endsWith("\n")) {
+	 * Fetches all the notes text from the slideshow, but not the slide text
-                                ret.append("\n");
+	 */
-                            }
+	public String getNotes() {
-                        }
+		return getText(false, true, false, false);
-                    }
-                }
 	}
 
-            for (HSLFSlide slide : _slides) {
+	@SuppressWarnings("unchecked")
-                String headerText = "";
+	public List<HSLFObjectShape> getOLEShapes() {
-                String footerText = "";
+		return (List<HSLFObjectShape>)delegate.getOLEShapes();
-                HeadersFooters hf = slide.getHeadersFooters();
-                if (hf != null) {
-                    if (hf.isHeaderVisible()) {
-                        headerText = safeLine(hf.getHeaderText());
-                    }
-                    if (hf.isFooterVisible()) {
-                        footerText = safeLine(hf.getFooterText());
-                    }
-                }
-                
-                // Slide header, if set
-                ret.append(headerText);
-
-                // Slide text
-                textRunsToText(ret, slide.getTextParagraphs());
-
-                // Table text
-                for (HSLFShape shape : slide.getShapes()){
-                    if (shape instanceof HSLFTable){
-                        extractTableText(ret, (HSLFTable)shape);
-                    }
-                }
-                // Slide footer, if set
-                ret.append(footerText);
-
-				// Comments, if requested and present
-				if (getCommentText) {
-					for (Comment comment : slide.getComments()) {
-						ret.append(comment.getAuthor() + " - " + comment.getText() + "\n");
-					}
-				}
-			}
-			if (getNoteText) {
-				ret.append('\n');
-			}
-		}
-
-		if (getNoteText) {
-			// Not currently using _notes, as that can have the notes of
-			// master sheets in. Grab Slide list, then work from there,
-			// but ensure no duplicates
-			Set<Integer> seenNotes = new HashSet<>();
-            String headerText = "";
-            String footerText = "";
-			HeadersFooters hf = _show.getNotesHeadersFooters();
-			if (hf != null) {
-			    if (hf.isHeaderVisible()) {
-			        headerText = safeLine(hf.getHeaderText());
-			    }
-			    if (hf.isFooterVisible()) {
-                    footerText = safeLine(hf.getFooterText());
-			    }
-			}
-			
-
-			for (HSLFSlide slide : _slides) {
-				HSLFNotes notes = slide.getNotes();
-				if (notes == null) {
-					continue;
-				}
-				Integer id = Integer.valueOf(notes._getSheetNumber());
-				if (seenNotes.contains(id)) {
-					continue;
-				}
-				seenNotes.add(id);
-
-				// Repeat the Notes header, if set
-				ret.append(headerText);
-
-				// Notes text
-				textRunsToText(ret, notes.getTextParagraphs());
-
-				// Repeat the notes footer, if set
-				ret.append(footerText);
-			}
-		}
-
-		return ret.toString();
-	}
-	
-	private static String safeLine(String text) {
-	    return (text == null) ? "" : (text+'\n');
-	}
-
-    private void extractTableText(StringBuffer ret, HSLFTable table) {
-        final int nrows = table.getNumberOfRows();
-        final int ncols = table.getNumberOfColumns();
-        for (int row = 0; row < nrows; row++){
-            for (int col = 0; col < ncols; col++){
-                HSLFTableCell cell = table.getCell(row, col);
-                //defensive null checks; don't know if they're necessary
-                if (cell != null){
-                    String txt = cell.getText();
-                    txt = (txt == null) ? "" : txt;
-                    ret.append(txt);
-                    if (col < ncols-1){
-                        ret.append('\t');
-                    }
-                }
-            }
-            ret.append('\n');
-        }
-    }
-    private void textRunsToText(StringBuffer ret, List<List<HSLFTextParagraph>> paragraphs) {
-        if (paragraphs==null) {
-            return;
-        }
-
-        for (List<HSLFTextParagraph> lp : paragraphs) {
-            ret.append(HSLFTextParagraph.getText(lp));
-            if (ret.length() > 0 && ret.charAt(ret.length()-1) != '\n') {
-                ret.append('\n');
-            }
-        }
 	}
 }

src/scratchpad/src/org/apache/poi/hslf/model/Comment.java

@@ -1,75 +0,0 @@
-/* ====================================================================
-   Licensed to the Apache Software Foundation (ASF) under one or more
-   contributor license agreements.  See the NOTICE file distributed with
-   this work for additional information regarding copyright ownership.
-   The ASF licenses this file to You under the Apache License, Version 2.0
-   (the "License"); you may not use this file except in compliance with
-   the License.  You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
-==================================================================== */
-
-package org.apache.poi.hslf.model;
-
-import org.apache.poi.hslf.record.Comment2000;
-
-/**
- *
- * @author Nick Burch
- */
-public final class Comment {
-	private Comment2000 _comment2000;
-
-	public Comment(Comment2000 comment2000) {
-		_comment2000 = comment2000;
-	}
-
-	protected Comment2000 getComment2000() {
-		return _comment2000;
-	}
-
-	/**
-	 * Get the Author of this comment
-	 */
-	public String getAuthor() {
-		return _comment2000.getAuthor();
-	}
-	/**
-	 * Set the Author of this comment
-	 */
-	public void setAuthor(String author) {
-		_comment2000.setAuthor(author);
-	}
-
-	/**
-	 * Get the Author's Initials of this comment
-	 */
-	public String getAuthorInitials() {
-		return _comment2000.getAuthorInitials();
-	}
-	/**
-	 * Set the Author's Initials of this comment
-	 */
-	public void setAuthorInitials(String initials) {
-		_comment2000.setAuthorInitials(initials);
-	}
-
-	/**
-	 * Get the text of this comment
-	 */
-	public String getText() {
-		return _comment2000.getText();
-	}
-	/**
-	 * Set the text of this comment
-	 */
-	public void setText(String text) {
-		_comment2000.setText(text);
-	}
-}

src/scratchpad/src/org/apache/poi/hslf/model/HeadersFooters.java

@@ -275,4 +275,11 @@ public final class HeadersFooters {
     private void setFlag(int type, boolean flag) {
         _container.getHeadersFootersAtom().setFlag(type, flag);
     }
+
+    /**
+     * @return true, if this is a ppt 2007 document and header/footer are stored as placeholder shapes
+     */
+    public boolean isPpt2007() {
+        return _ppt2007;
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/record/DocumentEncryptionAtom.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.util.Map;
 
+import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.poifs.crypt.CipherAlgorithm;
 import org.apache.poi.poifs.crypt.EncryptionInfo;
 import org.apache.poi.poifs.crypt.EncryptionMode;
@@ -46,15 +47,17 @@ public final class DocumentEncryptionAtom extends PositionDependentRecordAtom {
 	/**
 	 * For the Document Encryption Atom
 	 */
-	protected DocumentEncryptionAtom(byte[] source, int start, int len) throws IOException {
+	protected DocumentEncryptionAtom(byte[] source, int start, int len) {
 		// Get the header
 		_header = new byte[8];
 		System.arraycopy(source,start,_header,0,8);
 
 		ByteArrayInputStream bis = new ByteArrayInputStream(source, start+8, len-8);
-		LittleEndianInputStream leis = new LittleEndianInputStream(bis);
+		try (LittleEndianInputStream leis = new LittleEndianInputStream(bis)) {
 			ei = new EncryptionInfo(leis, EncryptionMode.cryptoAPI);
-		leis.close();
+		} catch (IOException e) {
+			throw new EncryptedDocumentException(e);
+		}
 	}
 
 	public DocumentEncryptionAtom() {

src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherClientDataRecord.java

@@ -20,7 +20,6 @@ package org.apache.poi.hslf.record;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 
 import org.apache.poi.ddf.EscherClientDataRecord;
@@ -49,12 +48,7 @@ public class HSLFEscherClientDataRecord extends EscherClientDataRecord {
     }
     
     public void removeChild(Class<? extends Record> childClass) {
-        Iterator<Record> iter = _childRecords.iterator();
+        _childRecords.removeIf(childClass::isInstance);
-        while (iter.hasNext()) {
-            if (childClass.isInstance(iter.next())) {
-                iter.remove();
-            }
-        }
     }
     
     public void addChild(Record childRecord) {
@@ -109,8 +103,10 @@ public class HSLFEscherClientDataRecord extends EscherClientDataRecord {
         _childRecords.clear();
         int offset = 0;
         while (offset < remainingData.length) {
-            Record r = Record.buildRecordAtOffset(remainingData, offset);
+            final Record r = Record.buildRecordAtOffset(remainingData, offset);
+            if (r != null) {
                 _childRecords.add(r);
+            }
             long rlen = LittleEndian.getUInt(remainingData,offset+4);
             offset += 8 + rlen;
         }

src/scratchpad/src/org/apache/poi/hslf/record/Record.java

@@ -24,6 +24,7 @@ import java.util.List;
 
 import org.apache.poi.hslf.exceptions.CorruptPowerPointFileException;
 import org.apache.poi.hslf.exceptions.HSLFException;
+import org.apache.poi.hslf.record.RecordTypes.RecordConstructor;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -122,15 +123,13 @@ public abstract class Record
 
 			// Abort if first record is of type 0000 and length FFFF,
 			//  as that's a sign of a screwed up record
-			if(pos == 0 && type == 0l && rleni == 0xffff) {
+			if(pos == 0 && type == 0L && rleni == 0xffff) {
 				throw new CorruptPowerPointFileException("Corrupt document - starts with record of type 0000 and length 0xFFFF");
 			}
 
 			Record r = createRecordForType(type,b,pos,8+rleni);
 			if(r != null) {
 				children.add(r);
-			} else {
-				// Record was horribly corrupt
 			}
 			pos += 8;
 			pos += rleni;
@@ -150,43 +149,32 @@ public abstract class Record
 	 *  passing in corrected lengths
 	 */
 	public static Record createRecordForType(long type, byte[] b, int start, int len) {
-		Record toReturn = null;
-
-		// Handle case of a corrupt last record, whose claimed length
-		//  would take us passed the end of the file
-		if(start + len > b.length) {
-			logger.log(POILogger.WARN, "Warning: Skipping record of type " + type + " at position " + start + " which claims to be longer than the file! (" + len + " vs " + (b.length-start) + ")");
-			return null;
-		}
-
 		// We use the RecordTypes class to provide us with the right
 		//  class to use for a given type
 		// A spot of reflection gets us the (byte[],int,int) constructor
 		// From there, we instanciate the class
 		// Any special record handling occurs once we have the class
-		Class<? extends Record> c = null;
+		RecordConstructor c = RecordTypes.forTypeID((short)type).recordConstructor;
-		try {
+		if (c == null) {
-			c = RecordTypes.forTypeID((short)type).handlingClass;
-			if(c == null) {
 			// How odd. RecordTypes normally substitutes in
 			//  a default handler class if it has heard of the record
 			//  type but there's no support for it. Explicitly request
 			//  that now
-				c = RecordTypes.UnknownRecordPlaceholder.handlingClass;
+			c = RecordTypes.UnknownRecordPlaceholder.recordConstructor;
 		}
 
-			// Grab the right constructor
+		final Record toReturn;
-			java.lang.reflect.Constructor<? extends Record> con = c.getDeclaredConstructor(new Class[] { byte[].class, Integer.TYPE, Integer.TYPE });
+		try {
-			// Instantiate
+			toReturn = c.apply(b, start, len);
-			toReturn = con.newInstance(new Object[] { b, start, len });
+		} catch(RuntimeException e) {
-		} catch(InstantiationException ie) {
+			// Handle case of a corrupt last record, whose claimed length
-			throw new HSLFException("Couldn't instantiate the class for type with id " + type + " on class " + c + " : " + ie, ie);
+			//  would take us passed the end of the file
-		} catch(java.lang.reflect.InvocationTargetException ite) {
+			if(start + len > b.length ) {
-			throw new HSLFException("Couldn't instantiate the class for type with id " + type + " on class " + c + " : " + ite + "\nCause was : " + ite.getCause(), ite);
+				logger.log(POILogger.WARN, "Warning: Skipping record of type " + type + " at position " + start + " which claims to be longer than the file! (" + len + " vs " + (b.length-start) + ")");
-		} catch(IllegalAccessException iae) {
+				return null;
-			throw new HSLFException("Couldn't access the constructor for type with id " + type + " on class " + c + " : " + iae, iae);
+			}
-		} catch(NoSuchMethodException nsme) {
+
-			throw new HSLFException("Couldn't access the constructor for type with id " + type + " on class " + c + " : " + nsme, nsme);
+			throw new HSLFException("Couldn't instantiate the class for type with id " + type + " on class " + c + " : " + e, e);
 		}
 
 		// Handling for special kinds of records follow

src/scratchpad/src/org/apache/poi/hslf/record/RecordTypes.java

@@ -29,141 +29,141 @@ import java.util.Map;
  */
 public enum RecordTypes {
     Unknown(0,null),
-    UnknownRecordPlaceholder(-1, UnknownRecordPlaceholder.class),
+    UnknownRecordPlaceholder(-1, UnknownRecordPlaceholder::new),
-    Document(1000,Document.class),
+    Document(1000,Document::new),
-    DocumentAtom(1001,DocumentAtom.class),
+    DocumentAtom(1001,DocumentAtom::new),
     EndDocument(1002,null),
-    Slide(1006,Slide.class),
+    Slide(1006,Slide::new),
-    SlideAtom(1007,SlideAtom.class),
+    SlideAtom(1007,SlideAtom::new),
-    Notes(1008,Notes.class),
+    Notes(1008,Notes::new),
-    NotesAtom(1009,NotesAtom.class),
+    NotesAtom(1009,NotesAtom::new),
-    Environment(1010,Environment.class),
+    Environment(1010,Environment::new),
-    SlidePersistAtom(1011,SlidePersistAtom.class),
+    SlidePersistAtom(1011,SlidePersistAtom::new),
     SSlideLayoutAtom(1015,null),
-    MainMaster(1016,MainMaster.class),
+    MainMaster(1016,MainMaster::new),
-    SSSlideInfoAtom(1017,SSSlideInfoAtom.class),
+    SSSlideInfoAtom(1017,SSSlideInfoAtom::new),
     SlideViewInfo(1018,null),
     GuideAtom(1019,null),
     ViewInfo(1020,null),
     ViewInfoAtom(1021,null),
     SlideViewInfoAtom(1022,null),
-    VBAInfo(1023,VBAInfoContainer.class),
+    VBAInfo(1023,VBAInfoContainer::new),
-    VBAInfoAtom(1024,VBAInfoAtom.class),
+    VBAInfoAtom(1024,VBAInfoAtom::new),
     SSDocInfoAtom(1025,null),
     Summary(1026,null),
     DocRoutingSlip(1030,null),
     OutlineViewInfo(1031,null),
     SorterViewInfo(1032,null),
-    ExObjList(1033,ExObjList.class),
+    ExObjList(1033,ExObjList::new),
-    ExObjListAtom(1034,ExObjListAtom.class),
+    ExObjListAtom(1034,ExObjListAtom::new),
-    PPDrawingGroup(1035,PPDrawingGroup.class),
+    PPDrawingGroup(1035,PPDrawingGroup::new),
-    PPDrawing(1036,PPDrawing.class),
+    PPDrawing(1036,PPDrawing::new),
     NamedShows(1040,null),
     NamedShow(1041,null),
     NamedShowSlides(1042,null),
     SheetProperties(1044,null),
     RoundTripCustomTableStyles12Atom(1064,null),
-    List(2000,DocInfoListContainer.class),
+    List(2000,DocInfoListContainer::new),
-    FontCollection(2005,FontCollection.class),
+    FontCollection(2005,FontCollection::new),
     BookmarkCollection(2019,null),
-    SoundCollection(2020,SoundCollection.class),
+    SoundCollection(2020,SoundCollection::new),
     SoundCollAtom(2021,null),
-    Sound(2022,Sound.class),
+    Sound(2022,Sound::new),
-    SoundData(2023,SoundData.class),
+    SoundData(2023,SoundData::new),
     BookmarkSeedAtom(2025,null),
-    ColorSchemeAtom(2032,ColorSchemeAtom.class),
+    ColorSchemeAtom(2032,ColorSchemeAtom::new),
-    ExObjRefAtom(3009,ExObjRefAtom.class),
+    ExObjRefAtom(3009,ExObjRefAtom::new),
-    OEPlaceholderAtom(3011,OEPlaceholderAtom.class),
+    OEPlaceholderAtom(3011,OEPlaceholderAtom::new),
     GPopublicintAtom(3024,null),
     GRatioAtom(3031,null),
-    OutlineTextRefAtom(3998,OutlineTextRefAtom.class),
+    OutlineTextRefAtom(3998,OutlineTextRefAtom::new),
-    TextHeaderAtom(3999,TextHeaderAtom.class),
+    TextHeaderAtom(3999,TextHeaderAtom::new),
-    TextCharsAtom(4000,TextCharsAtom.class),
+    TextCharsAtom(4000,TextCharsAtom::new),
-    StyleTextPropAtom(4001, StyleTextPropAtom.class),//0x0fa1 RT_StyleTextPropAtom
+    StyleTextPropAtom(4001, StyleTextPropAtom::new),//0x0fa1 RT_StyleTextPropAtom
-    MasterTextPropAtom(4002, MasterTextPropAtom.class),
+    MasterTextPropAtom(4002, MasterTextPropAtom::new),
-    TxMasterStyleAtom(4003,TxMasterStyleAtom.class),
+    TxMasterStyleAtom(4003,TxMasterStyleAtom::new),
     TxCFStyleAtom(4004,null),
     TxPFStyleAtom(4005,null),
-    TextRulerAtom(4006,TextRulerAtom.class),
+    TextRulerAtom(4006,TextRulerAtom::new),
     TextBookmarkAtom(4007,null),
-    TextBytesAtom(4008,TextBytesAtom.class),
+    TextBytesAtom(4008,TextBytesAtom::new),
     TxSIStyleAtom(4009,null),
-    TextSpecInfoAtom(4010, TextSpecInfoAtom.class),
+    TextSpecInfoAtom(4010, TextSpecInfoAtom::new),
     DefaultRulerAtom(4011,null),
-    StyleTextProp9Atom(4012, StyleTextProp9Atom.class), //0x0FAC RT_StyleTextProp9Atom
+    StyleTextProp9Atom(4012, StyleTextProp9Atom::new), //0x0FAC RT_StyleTextProp9Atom
-    FontEntityAtom(4023,FontEntityAtom.class),
+    FontEntityAtom(4023,FontEntityAtom::new),
     FontEmbeddedData(4024,null),
-    CString(4026,CString.class),
+    CString(4026,CString::new),
     MetaFile(4033,null),
-    ExOleObjAtom(4035,ExOleObjAtom.class),
+    ExOleObjAtom(4035,ExOleObjAtom::new),
     SrKinsoku(4040,null),
-    HandOut(4041,DummyPositionSensitiveRecordWithChildren.class),
+    HandOut(4041,DummyPositionSensitiveRecordWithChildren::new),
-    ExEmbed(4044,ExEmbed.class),
+    ExEmbed(4044,ExEmbed::new),
-    ExEmbedAtom(4045,ExEmbedAtom.class),
+    ExEmbedAtom(4045,ExEmbedAtom::new),
     ExLink(4046,null),
     BookmarkEntityAtom(4048,null),
     ExLinkAtom(4049,null),
     SrKinsokuAtom(4050,null),
-    ExHyperlinkAtom(4051,ExHyperlinkAtom.class),
+    ExHyperlinkAtom(4051,ExHyperlinkAtom::new),
-    ExHyperlink(4055,ExHyperlink.class),
+    ExHyperlink(4055,ExHyperlink::new),
     SlideNumberMCAtom(4056,null),
-    HeadersFooters(4057,HeadersFootersContainer.class),
+    HeadersFooters(4057,HeadersFootersContainer::new),
-    HeadersFootersAtom(4058,HeadersFootersAtom.class),
+    HeadersFootersAtom(4058,HeadersFootersAtom::new),
-    TxInteractiveInfoAtom(4063,TxInteractiveInfoAtom.class),
+    TxInteractiveInfoAtom(4063,TxInteractiveInfoAtom::new),
     CharFormatAtom(4066,null),
     ParaFormatAtom(4067,null),
     RecolorInfoAtom(4071,null),
     ExQuickTimeMovie(4074,null),
     ExQuickTimeMovieData(4075,null),
-    ExControl(4078,ExControl.class),
+    ExControl(4078,ExControl::new),
-    SlideListWithText(4080,SlideListWithText.class),
+    SlideListWithText(4080,SlideListWithText::new),
-    InteractiveInfo(4082,InteractiveInfo.class),
+    InteractiveInfo(4082,InteractiveInfo::new),
-    InteractiveInfoAtom(4083,InteractiveInfoAtom.class),
+    InteractiveInfoAtom(4083,InteractiveInfoAtom::new),
-    UserEditAtom(4085,UserEditAtom.class),
+    UserEditAtom(4085,UserEditAtom::new),
     CurrentUserAtom(4086,null),
     DateTimeMCAtom(4087,null),
     GenericDateMCAtom(4088,null),
     FooterMCAtom(4090,null),
-    ExControlAtom(4091,ExControlAtom.class),
+    ExControlAtom(4091,ExControlAtom::new),
-    ExMediaAtom(4100,ExMediaAtom.class),
+    ExMediaAtom(4100,ExMediaAtom::new),
-    ExVideoContainer(4101,ExVideoContainer.class),
+    ExVideoContainer(4101,ExVideoContainer::new),
-    ExAviMovie(4102,ExAviMovie.class),
+    ExAviMovie(4102,ExAviMovie::new),
-    ExMCIMovie(4103,ExMCIMovie.class),
+    ExMCIMovie(4103,ExMCIMovie::new),
     ExMIDIAudio(4109,null),
     ExCDAudio(4110,null),
     ExWAVAudioEmbedded(4111,null),
     ExWAVAudioLink(4112,null),
-    ExOleObjStg(4113,ExOleObjStg.class),
+    ExOleObjStg(4113,ExOleObjStg::new),
     ExCDAudioAtom(4114,null),
     ExWAVAudioEmbeddedAtom(4115,null),
-    AnimationInfo(4116,AnimationInfo.class),
+    AnimationInfo(4116,AnimationInfo::new),
-    AnimationInfoAtom(4081,AnimationInfoAtom.class),
+    AnimationInfoAtom(4081,AnimationInfoAtom::new),
     RTFDateTimeMCAtom(4117,null),
-    ProgTags(5000,DummyPositionSensitiveRecordWithChildren.class),
+    ProgTags(5000,DummyPositionSensitiveRecordWithChildren::new),
     ProgStringTag(5001,null),
-    ProgBinaryTag(5002,DummyPositionSensitiveRecordWithChildren.class),
+    ProgBinaryTag(5002,DummyPositionSensitiveRecordWithChildren::new),
-    BinaryTagData(5003, BinaryTagDataBlob.class),//0x138b RT_BinaryTagDataBlob
+    BinaryTagData(5003, BinaryTagDataBlob::new),//0x138b RT_BinaryTagDataBlob
     PrpublicintOptions(6000,null),
-    PersistPtrFullBlock(6001,PersistPtrHolder.class),
+    PersistPtrFullBlock(6001,PersistPtrHolder::new),
-    PersistPtrIncrementalBlock(6002,PersistPtrHolder.class),
+    PersistPtrIncrementalBlock(6002,PersistPtrHolder::new),
     GScalingAtom(10001,null),
     GRColorAtom(10002,null),
 
     // Records ~12000 seem to be related to the Comments used in PPT 2000/XP
     // (Comments in PPT97 are normal Escher text boxes)
-    Comment2000(12000,Comment2000.class),
+    Comment2000(12000,Comment2000::new),
-    Comment2000Atom(12001,Comment2000Atom.class),
+    Comment2000Atom(12001,Comment2000Atom::new),
     Comment2000Summary(12004,null),
     Comment2000SummaryAtom(12005,null),
 
     // Records ~12050 seem to be related to Document Encryption
-    DocumentEncryptionAtom(12052,DocumentEncryptionAtom.class),
+    DocumentEncryptionAtom(12052,DocumentEncryptionAtom::new),
 
     OriginalMainMasterId(1052,null),
     CompositeMasterId(1052,null),
     RoundTripContentMasterInfo12(1054,null),
     RoundTripShapeId12(1055,null),
-    RoundTripHFPlaceholder12(1056,RoundTripHFPlaceholder12.class),
+    RoundTripHFPlaceholder12(1056,RoundTripHFPlaceholder12::new),
     RoundTripContentMasterId(1058,null),
     RoundTripOArtTextStyles12(1059,null),
     RoundTripShapeCheckSumForCustomLayouts12(1062,null),
@@ -207,6 +207,11 @@ public enum RecordTypes {
     // same as EscherTertiaryOptRecord.RECORD_ID
     EscherUserDefined(0xf122,null);
 
+    @FunctionalInterface
+    public interface RecordConstructor<T extends Record> {
+         T apply(byte[] source, int start, int len);
+    }
+
     private static final Map<Short,RecordTypes> LOOKUP;
 
     static {
@@ -217,85 +222,15 @@ public enum RecordTypes {
     }    
     
     public final short typeID;
-    public final Class<? extends Record> handlingClass;
+    public final RecordConstructor recordConstructor;
 
-    private RecordTypes(int typeID, Class<? extends Record> handlingClass) {
+    RecordTypes(int typeID, RecordConstructor recordConstructor) {
         this.typeID = (short)typeID;
-        this.handlingClass = handlingClass;
+        this.recordConstructor = recordConstructor;
     }
 
     public static RecordTypes forTypeID(int typeID) {
         RecordTypes rt = LOOKUP.get((short)typeID);
         return (rt != null) ? rt : UnknownRecordPlaceholder;
     }
-
-
-
-    /**
-     * Returns name of the record by its type
-     *
-     * @param type section of the record header
-     * @return name of the record
-     */
-//    public static String recordName(int type) {
-//        String name = typeToName.get(Integer.valueOf(type));
-//        return (name == null) ? ("Unknown" + type) : name;
-//    }
-
-    /**
-     * Returns the class handling a record by its type.
-	 * If given an un-handled PowerPoint record, will return a dummy
-	 *  placeholder class. If given an unknown PowerPoint record, or
-	 *  and Escher record, will return null.
-     *
-     * @param type section of the record header
-     * @return class to handle the record, or null if an unknown (eg Escher) record
-     */
-//	public static Class<? extends Record> recordHandlingClass(int type) {
-//		Class<? extends Record> c = typeToClass.get(Integer.valueOf(type));
-//		return c;
-//	}
-//
-//    static {
-//		typeToName = new HashMap<Integer,String>();
-//		typeToClass = new HashMap<Integer,Class<? extends Record>>();
-//        try {
-//            Field[] f = RecordTypes.class.getFields();
-//            for (int i = 0; i < f.length; i++){
-//               Object val = f[i].get(null);
-//
-//               // Escher record, only store ID -> Name
-//               if (val instanceof Integer) {
-//                  typeToName.put((Integer)val, f[i].getName());
-//               }
-//               // PowerPoint record, store ID -> Name and ID -> Class
-//               if (val instanceof Type) {
-//                  Type t = (Type)val;
-//                  Class<? extends Record> c = t.handlingClass;
-//                  Integer id = Integer.valueOf(t.typeID);
-//                  if(c == null) { c = UnknownRecordPlaceholder.class; }
-//
-//                  typeToName.put(id, f[i].getName());
-//                  typeToClass.put(id, c);
-//               }
-//            }
-//        } catch (IllegalAccessException e){
-//            throw new HSLFException("Failed to initialize records types");
-//        }
-//    }
-
-
-	/**
-	 * Wrapper for the details of a PowerPoint or Escher record type.
-	 * Contains both the type, and the handling class (if any), and
-	 *  offers methods to get either back out.
-	 */
-//	public static class Type {
-//		public final int typeID;
-//		public final Class<? extends Record> handlingClass;
-//		public Type(int typeID, Class<? extends Record> handlingClass) {
-//			this.typeID = typeID;
-//			this.handlingClass = handlingClass;
-//		}
-//	}
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFComment.java

@@ -0,0 +1,104 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hslf.usermodel;
+
+import org.apache.poi.hslf.record.Comment2000;
+import org.apache.poi.sl.usermodel.Comment;
+import org.apache.poi.util.Units;
+
+import java.awt.geom.Point2D;
+import java.util.Date;
+
+public final class HSLFComment implements Comment {
+    private Comment2000 _comment2000;
+
+    public HSLFComment(Comment2000 comment2000) {
+        _comment2000 = comment2000;
+    }
+
+    protected Comment2000 getComment2000() {
+        return _comment2000;
+    }
+
+    /**
+     * Get the Author of this comment
+     */
+    public String getAuthor() {
+        return _comment2000.getAuthor();
+    }
+
+    /**
+     * Set the Author of this comment
+     */
+    public void setAuthor(String author) {
+        _comment2000.setAuthor(author);
+    }
+
+    /**
+     * Get the Author's Initials of this comment
+     */
+    public String getAuthorInitials() {
+        return _comment2000.getAuthorInitials();
+    }
+
+    /**
+     * Set the Author's Initials of this comment
+     */
+    public void setAuthorInitials(String initials) {
+        _comment2000.setAuthorInitials(initials);
+    }
+
+    /**
+     * Get the text of this comment
+     */
+    public String getText() {
+        return _comment2000.getText();
+    }
+
+    /**
+     * Set the text of this comment
+     */
+    public void setText(String text) {
+        _comment2000.setText(text);
+    }
+
+    @Override
+    public Date getDate() {
+        return _comment2000.getComment2000Atom().getDate();
+    }
+
+    @Override
+    public void setDate(Date date) {
+        _comment2000.getComment2000Atom().setDate(date);
+    }
+
+    @Override
+    public Point2D getOffset() {
+        final double x = Units.masterToPoints(_comment2000.getComment2000Atom().getXOffset());
+        final double y = Units.masterToPoints(_comment2000.getComment2000Atom().getYOffset());
+        return new Point2D.Double(x, y);
+    }
+
+    @Override
+    public void setOffset(Point2D offset) {
+        final int x = Units.pointsToMaster(offset.getX());
+        final int y = Units.pointsToMaster(offset.getY());
+        _comment2000.getComment2000Atom().setXOffset(x);
+        _comment2000.getComment2000Atom().setYOffset(y);
+    }
+}

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFMasterSheet.java

@@ -21,6 +21,8 @@ import org.apache.poi.hslf.model.textproperties.TextPropCollection;
 import org.apache.poi.hslf.record.SheetContainer;
 import org.apache.poi.hslf.record.TextHeaderAtom;
 import org.apache.poi.sl.usermodel.MasterSheet;
+import org.apache.poi.sl.usermodel.SimpleShape;
+import org.apache.poi.util.Removal;
 
 /**
  * The superclass of all master sheets - Slide masters, Notes masters, etc.
@@ -52,11 +54,13 @@ public abstract class HSLFMasterSheet extends HSLFSheet implements MasterSheet<H
      *
      *
      * @return true if the shape is a placeholder
+     * 
+     * @deprecated use {@link SimpleShape#isPlaceholder()}
      */
+    @Deprecated
+    @Removal(version="4.1.0")
     public static boolean isPlaceholder(HSLFShape shape){
-        if(!(shape instanceof HSLFTextShape)) return false;
+        return shape instanceof SimpleShape 
-
+            && ((SimpleShape<?,?>)shape).isPlaceholder();
-        HSLFTextShape tx = (HSLFTextShape)shape;
-        return tx.getPlaceholderAtom() != null;
     }
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFNotes.java

@@ -20,7 +20,10 @@ package org.apache.poi.hslf.usermodel;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.poi.hslf.model.HeadersFooters;
+import org.apache.poi.hslf.record.HeadersFootersContainer;
 import org.apache.poi.sl.usermodel.Notes;
+import org.apache.poi.sl.usermodel.Placeholder;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
 
@@ -72,4 +75,28 @@ public final class HSLFNotes extends HSLFSheet implements Notes<HSLFShape,HSLFTe
     public HSLFMasterSheet getMasterSheet() {
         return null;
     }
+
+    /**
+     * Header / Footer settings for this slide.
+     *
+     * @return Header / Footer settings for this slide
+     */
+    @Override
+    public HeadersFooters getHeadersFooters() {
+        return new HeadersFooters(this, HeadersFootersContainer.NotesHeadersFootersContainer);
+    }
+
+
+    @Override
+    public HSLFPlaceholderDetails getPlaceholderDetails(Placeholder placeholder) {
+        if (placeholder == null) {
+            return null;
+        }
+
+        if (placeholder == Placeholder.HEADER || placeholder == Placeholder.FOOTER) {
+            return new HSLFPlaceholderDetails(this, placeholder);
+        } else {
+            return super.getPlaceholderDetails(placeholder);
+        }
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFPlaceholderDetails.java

@@ -0,0 +1,157 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hslf.usermodel;
+
+import org.apache.poi.hslf.model.HeadersFooters;
+import org.apache.poi.sl.usermodel.Placeholder;
+import org.apache.poi.sl.usermodel.PlaceholderDetails;
+
+/**
+ * Extended placeholder details for HSLF sheets - mainly for headers and footers
+ *
+ * @since POI 4.0.0
+ */
+public class HSLFPlaceholderDetails implements PlaceholderDetails {
+    private final HSLFSheet sheet;
+    private final Placeholder placeholder;
+
+    HSLFPlaceholderDetails(final HSLFSheet sheet, final Placeholder placeholder) {
+        this.sheet = sheet;
+        this.placeholder = placeholder;
+    }
+
+
+    public boolean isVisible() {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return false;
+        }
+
+        final HeadersFooters headersFooters = sheet.getHeadersFooters();
+
+        switch (ph) {
+            case HEADER:
+                return headersFooters.isHeaderVisible();
+            case FOOTER:
+                return headersFooters.isFooterVisible();
+            case DATETIME:
+                return headersFooters.isDateTimeVisible();
+            case TITLE:
+                return headersFooters.isHeaderVisible();
+            case SLIDE_NUMBER:
+                return headersFooters.isSlideNumberVisible();
+            default:
+                return false;
+        }
+    }
+
+    public void setVisible(final boolean isVisible) {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return;
+        }
+
+        final HeadersFooters headersFooters = sheet.getHeadersFooters();
+
+        switch (ph) {
+            case TITLE:
+            case HEADER:
+                headersFooters.setHeaderVisible(isVisible);
+                break;
+            case FOOTER:
+                headersFooters.setFooterVisible(isVisible);
+                break;
+            case DATETIME:
+                headersFooters.setDateTimeVisible(isVisible);
+                break;
+            case SLIDE_NUMBER:
+                headersFooters.setSlideNumberVisible(isVisible);
+                break;
+        }
+    }
+
+    @Override
+    public Placeholder getPlaceholder() {
+        return placeholder;
+    }
+
+    @Override
+    public void setPlaceholder(Placeholder placeholder) {
+        throw new UnsupportedOperationException("Only sub class(es) of HSLFPlaceholderDetails allow setting the placeholder");
+    }
+
+    @Override
+    public PlaceholderSize getSize() {
+        return PlaceholderSize.full;
+    }
+
+    @Override
+    public void setSize(PlaceholderSize size) {
+        throw new UnsupportedOperationException("Only sub class(es) of HSLFPlaceholderDetails allow setting the size");
+    }
+
+    @Override
+    public String getText() {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return null;
+        }
+
+        final HeadersFooters headersFooters = sheet.getHeadersFooters();
+
+        switch (ph) {
+            case TITLE:
+            case HEADER:
+                return headersFooters.getHeaderText();
+            case FOOTER:
+                return headersFooters.getFooterText();
+            case DATETIME:
+                return headersFooters.getDateTimeText();
+            case SLIDE_NUMBER:
+            default:
+                return null;
+        }
+    }
+
+    @Override
+    public void setText(final String text) {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return;
+        }
+
+        final HeadersFooters headersFooters = sheet.getHeadersFooters();
+
+        switch (ph) {
+            case TITLE:
+            case HEADER:
+                headersFooters.setHeaderText(text);
+                break;
+            case FOOTER:
+                headersFooters.setFootersText(text);
+                break;
+            case DATETIME:
+                headersFooters.setDateTimeText(text);
+                break;
+            case SLIDE_NUMBER:
+            default:
+                break;
+        }
+
+    }
+}

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java

@@ -0,0 +1,240 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hslf.usermodel;
+
+import org.apache.poi.ddf.EscherProperties;
+import org.apache.poi.ddf.EscherSpRecord;
+import org.apache.poi.hslf.exceptions.HSLFException;
+import org.apache.poi.hslf.record.HSLFEscherClientDataRecord;
+import org.apache.poi.hslf.record.OEPlaceholderAtom;
+import org.apache.poi.hslf.record.Record;
+import org.apache.poi.hslf.record.RoundTripHFPlaceholder12;
+import org.apache.poi.sl.usermodel.MasterSheet;
+import org.apache.poi.sl.usermodel.Placeholder;
+
+/**
+ * Extended placeholder details for HSLF shapes
+ *
+ * @since POI 4.0.0
+ */
+public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails {
+    private enum PlaceholderContainer {
+        slide, master, notes, notesMaster
+    }
+
+    private final PlaceholderContainer source;
+    final HSLFSimpleShape shape;
+    private OEPlaceholderAtom oePlaceholderAtom;
+    private RoundTripHFPlaceholder12 roundTripHFPlaceholder12;
+
+
+    HSLFShapePlaceholderDetails(final HSLFSimpleShape shape) {
+        super(shape.getSheet(), null);
+
+        this.shape = shape;
+
+        final HSLFSheet sheet = shape.getSheet();
+        if (sheet instanceof HSLFSlideMaster) {
+            source = PlaceholderContainer.master;
+        } else if (sheet instanceof HSLFNotes) {
+            source = PlaceholderContainer.notes;
+        } else if (sheet instanceof MasterSheet) {
+            // notes master aren't yet supported ...
+            source = PlaceholderContainer.notesMaster;
+        } else {
+            source = PlaceholderContainer.slide;
+        }
+    }
+
+    public Placeholder getPlaceholder() {
+        updatePlaceholderAtom(false);
+        final int phId;
+        if (oePlaceholderAtom != null) {
+            phId = oePlaceholderAtom.getPlaceholderId();
+        } else if (roundTripHFPlaceholder12 != null) {
+            phId = roundTripHFPlaceholder12.getPlaceholderId();
+        } else {
+            return null;
+        }
+
+        switch (source) {
+        case slide:
+            return Placeholder.lookupNativeSlide(phId);
+        default:
+        case master:
+            return Placeholder.lookupNativeSlideMaster(phId);
+        case notes:
+            return Placeholder.lookupNativeNotes(phId);
+        case notesMaster:
+            return Placeholder.lookupNativeNotesMaster(phId);
+        }
+    }
+
+    public void setPlaceholder(final Placeholder placeholder) {
+        final EscherSpRecord spRecord = shape.getEscherChild(EscherSpRecord.RECORD_ID);
+        int flags = spRecord.getFlags();
+        if (placeholder == null) {
+            flags ^= EscherSpRecord.FLAG_HAVEMASTER;
+        } else {
+            flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER;
+        }
+        spRecord.setFlags(flags);
+
+        // Placeholders can't be grouped
+        shape.setEscherProperty(EscherProperties.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144));
+
+        if (placeholder == null) {
+            removePlaceholder();
+            return;
+        }
+
+        // init client data
+        updatePlaceholderAtom(true);
+
+        final byte phId = getPlaceholderId(placeholder);
+        oePlaceholderAtom.setPlaceholderId(phId);
+        roundTripHFPlaceholder12.setPlaceholderId(phId);
+    }
+
+    public PlaceholderSize getSize() {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null) {
+            return null;
+        }
+
+        final int size = (oePlaceholderAtom != null) 
+            ? oePlaceholderAtom.getPlaceholderSize()
+            : OEPlaceholderAtom.PLACEHOLDER_HALFSIZE;
+        
+        switch (size) {
+        case OEPlaceholderAtom.PLACEHOLDER_FULLSIZE:
+            return PlaceholderSize.full;
+        default:
+        case OEPlaceholderAtom.PLACEHOLDER_HALFSIZE:
+            return PlaceholderSize.half;
+        case OEPlaceholderAtom.PLACEHOLDER_QUARTSIZE:
+            return PlaceholderSize.quarter;
+        }
+    }
+
+    public void setSize(final PlaceholderSize size) {
+        final Placeholder ph = getPlaceholder();
+        if (ph == null || size == null) {
+            return;
+        }
+        updatePlaceholderAtom(true);
+        
+        final byte ph_size;
+        switch (size) {
+        case full:
+            ph_size = OEPlaceholderAtom.PLACEHOLDER_FULLSIZE;
+            break;
+        default:
+        case half:
+            ph_size = OEPlaceholderAtom.PLACEHOLDER_HALFSIZE;
+            break;
+        case quarter:
+            ph_size = OEPlaceholderAtom.PLACEHOLDER_QUARTSIZE;
+            break;
+        }
+        oePlaceholderAtom.setPlaceholderSize(ph_size);
+    }
+
+    private byte getPlaceholderId(final Placeholder placeholder) {
+        /*
+         * Extract from MSDN:
+         *
+         * There is a special case when the placeholder does not have a position in the layout.
+         * This occurs when the user has moved the placeholder from its original position.
+         * In this case the placeholder ID is -1.
+         */
+        final byte phId;
+        switch (source) {
+            default:
+            case slide:
+                phId = (byte)placeholder.nativeSlideId;
+                break;
+            case master:
+                phId = (byte)placeholder.nativeSlideMasterId;
+                break;
+            case notes:
+                phId = (byte)placeholder.nativeNotesId;
+                break;
+            case notesMaster:
+                phId = (byte)placeholder.nativeNotesMasterId;
+                break;
+        }
+
+        if (phId == -2) {
+            throw new HSLFException("Placeholder "+placeholder.name()+" not supported for this sheet type ("+shape.getSheet().getClass()+")");
+        }
+
+        return phId;
+    }
+
+    private void removePlaceholder() {
+        final HSLFEscherClientDataRecord clientData = shape.getClientData(false);
+        if (clientData != null) {
+            clientData.removeChild(OEPlaceholderAtom.class);
+            clientData.removeChild(RoundTripHFPlaceholder12.class);
+            // remove client data if the placeholder was the only child to be carried
+            if (clientData.getChildRecords().isEmpty()) {
+                shape.getSpContainer().removeChildRecord(clientData);
+            }
+        }
+        oePlaceholderAtom = null;
+        roundTripHFPlaceholder12 = null;
+    }
+
+    private void updatePlaceholderAtom(final boolean create) {
+        final HSLFEscherClientDataRecord clientData = shape.getClientData(create);
+        if (clientData == null) {
+            oePlaceholderAtom = null;
+            roundTripHFPlaceholder12 = null;
+            if (!create) {
+                return;
+            }
+            throw new HSLFException("Placeholder aren't allowed for shape type: " + shape.getClass().getSimpleName());
+        }
+
+        for (Record r : clientData.getHSLFChildRecords()) {
+            if (r instanceof OEPlaceholderAtom) {
+                oePlaceholderAtom = (OEPlaceholderAtom)r;
+            } else if (r instanceof RoundTripHFPlaceholder12) {
+                //special case for files saved in Office 2007
+                roundTripHFPlaceholder12 = (RoundTripHFPlaceholder12)r;
+            }
+        }
+
+        if (!create) {
+            return;
+        }
+
+        if (oePlaceholderAtom == null) {
+            oePlaceholderAtom = new OEPlaceholderAtom();
+            oePlaceholderAtom.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE);
+            // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id
+            oePlaceholderAtom.setPlacementId(-1);
+            clientData.addChild(oePlaceholderAtom);
+        }
+        if (roundTripHFPlaceholder12 == null) {
+            roundTripHFPlaceholder12 = new RoundTripHFPlaceholder12();
+            clientData.addChild(roundTripHFPlaceholder12);
+        }
+    }
+}

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java

@@ -28,8 +28,10 @@ import org.apache.poi.ddf.EscherDgRecord;
 import org.apache.poi.ddf.EscherDggRecord;
 import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.hslf.exceptions.HSLFException;
+import org.apache.poi.hslf.model.HeadersFooters;
 import org.apache.poi.hslf.record.CString;
 import org.apache.poi.hslf.record.ColorSchemeAtom;
+import org.apache.poi.hslf.record.HeadersFootersContainer;
 import org.apache.poi.hslf.record.PPDrawing;
 import org.apache.poi.hslf.record.RecordContainer;
 import org.apache.poi.hslf.record.RecordTypes;
@@ -453,4 +455,20 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet<HSLFShape,H
         addShape(s);
         return s;
     }
+    
+    /**
+     * Header / Footer settings for this slide.
+     *
+     * @return Header / Footer settings for this slide
+     */
+    public HeadersFooters getHeadersFooters() {
+        return new HeadersFooters(this, HeadersFootersContainer.SlideHeadersFootersContainer);
+    }
+
+
+    @Override
+    public HSLFPlaceholderDetails getPlaceholderDetails(Placeholder placeholder) {
+        final HSLFSimpleShape ph = getPlaceholder(placeholder);
+        return (ph == null) ? null : new HSLFShapePlaceholderDetails(ph);
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java

@@ -18,7 +18,6 @@
 package org.apache.poi.hslf.usermodel;
 
 import java.awt.Color;
-import java.util.List;
 
 import org.apache.poi.ddf.AbstractEscherOptRecord;
 import org.apache.poi.ddf.EscherChildAnchorRecord;
@@ -31,10 +30,6 @@ import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.ddf.EscherSimpleProperty;
 import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.hslf.exceptions.HSLFException;
-import org.apache.poi.hslf.record.HSLFEscherClientDataRecord;
-import org.apache.poi.hslf.record.OEPlaceholderAtom;
-import org.apache.poi.hslf.record.Record;
-import org.apache.poi.hslf.record.RoundTripHFPlaceholder12;
 import org.apache.poi.sl.draw.DrawPaint;
 import org.apache.poi.sl.draw.geom.CustomGeometry;
 import org.apache.poi.sl.draw.geom.Guide;
@@ -42,7 +37,6 @@ import org.apache.poi.sl.draw.geom.PresetGeometries;
 import org.apache.poi.sl.usermodel.LineDecoration;
 import org.apache.poi.sl.usermodel.LineDecoration.DecorationShape;
 import org.apache.poi.sl.usermodel.LineDecoration.DecorationSize;
-import org.apache.poi.sl.usermodel.MasterSheet;
 import org.apache.poi.sl.usermodel.PaintStyle;
 import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
 import org.apache.poi.sl.usermodel.Placeholder;
@@ -540,145 +534,20 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H
         };
     }
 
+    @Override
+    public HSLFShapePlaceholderDetails getPlaceholderDetails() {
+        return new HSLFShapePlaceholderDetails(this);
+    }
+    
+    
     @Override
     public Placeholder getPlaceholder() {
-        List<? extends Record> clRecords = getClientRecords();
+        return getPlaceholderDetails().getPlaceholder();
-        if (clRecords == null) {
-            return null;
-        }
-        int phSource;
-        HSLFSheet sheet = getSheet();
-        if (sheet instanceof HSLFSlideMaster) {
-            phSource = 1;
-        } else if (sheet instanceof HSLFNotes) {
-            phSource = 2;
-        } else if (sheet instanceof MasterSheet) {
-            // notes master aren't yet supported ...
-            phSource = 3;
-        } else {
-            phSource = 0;
-        }
-        
-        for (Record r : clRecords) {
-            int phId;
-            if (r instanceof OEPlaceholderAtom) {
-                phId = ((OEPlaceholderAtom)r).getPlaceholderId();
-            } else if (r instanceof RoundTripHFPlaceholder12) {
-                //special case for files saved in Office 2007
-                phId = ((RoundTripHFPlaceholder12)r).getPlaceholderId();
-            } else {
-                continue;
-            }
-
-            switch (phSource) {
-            case 0:
-                return Placeholder.lookupNativeSlide(phId);
-            default:
-            case 1:
-                return Placeholder.lookupNativeSlideMaster(phId);
-            case 2:
-                return Placeholder.lookupNativeNotes(phId);
-            case 3:
-                return Placeholder.lookupNativeNotesMaster(phId);
-            }
-        }
-
-        return null;
     }
 
     @Override
     public void setPlaceholder(Placeholder placeholder) {
-        EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
+        getPlaceholderDetails().setPlaceholder(placeholder);
-        int flags = spRecord.getFlags();
-        if (placeholder == null) {
-            flags ^= EscherSpRecord.FLAG_HAVEMASTER;
-        } else {
-            flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER;
-        }
-        spRecord.setFlags(flags);
-
-        // Placeholders can't be grouped
-        setEscherProperty(EscherProperties.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144));
-
-        HSLFEscherClientDataRecord clientData = getClientData(false);
-        if (placeholder == null) {
-            if (clientData != null) {
-                clientData.removeChild(OEPlaceholderAtom.class);
-                clientData.removeChild(RoundTripHFPlaceholder12.class);
-                // remove client data if the placeholder was the only child to be carried
-                if (clientData.getChildRecords().isEmpty()) {
-                    getSpContainer().removeChildRecord(clientData);
-                }
-            }
-            return;
-        }
-
-        if (clientData == null) {
-            clientData = getClientData(true);
-        }
-
-        // OEPlaceholderAtom tells powerpoint that this shape is a placeholder
-        OEPlaceholderAtom oep = null;
-        RoundTripHFPlaceholder12 rtp = null;
-        for (Record r : clientData.getHSLFChildRecords()) {
-            if (r instanceof OEPlaceholderAtom) {
-                oep = (OEPlaceholderAtom)r;
-                break;
-            }
-            if (r instanceof RoundTripHFPlaceholder12) {
-                rtp = (RoundTripHFPlaceholder12)r;
-                break;
-            }
-        }
-
-        /**
-         * Extract from MSDN:
-         *
-         * There is a special case when the placeholder does not have a position in the layout.
-         * This occurs when the user has moved the placeholder from its original position.
-         * In this case the placeholder ID is -1.
-         */
-        byte phId;
-        HSLFSheet sheet = getSheet();
-        // TODO: implement/switch NotesMaster
-        if (sheet instanceof HSLFSlideMaster) {
-            phId = (byte)placeholder.nativeSlideMasterId;
-        } else if (sheet instanceof HSLFNotes) {
-            phId = (byte)placeholder.nativeNotesId;
-        } else {
-            phId = (byte)placeholder.nativeSlideId;
-        }
-
-        if (phId == -2) {
-            throw new HSLFException("Placeholder "+placeholder.name()+" not supported for this sheet type ("+sheet.getClass()+")");
-        }
-
-        switch (placeholder) {
-            case HEADER:
-            case FOOTER:
-                if (rtp == null) {
-                    rtp = new RoundTripHFPlaceholder12();
-                    rtp.setPlaceholderId(phId);
-                    clientData.addChild(rtp);
-                }
-                if (oep != null) {
-                    clientData.removeChild(OEPlaceholderAtom.class);
-                }
-                break;
-            default:
-                if (rtp != null) {
-                    clientData.removeChild(RoundTripHFPlaceholder12.class);
-                }
-                if (oep == null) {
-                    oep = new OEPlaceholderAtom();
-                    oep.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE);
-                    // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id
-                    oep.setPlacementId(-1);
-                    oep.setPlaceholderId(phId);
-                    clientData.addChild(oep);
-                }
-                break;
-        }
     }
 
 
@@ -727,4 +596,10 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H
     protected void setHyperlink(HSLFHyperlink link) {
         _hyperlink = link;
     }
+    
+    @Override
+    public boolean isPlaceholder() {
+        // currently we only identify TextShapes as placeholders
+        return false;
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlide.java

@@ -26,12 +26,12 @@ import org.apache.poi.ddf.EscherDgRecord;
 import org.apache.poi.ddf.EscherDggRecord;
 import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.hslf.exceptions.HSLFException;
-import org.apache.poi.hslf.model.Comment;
 import org.apache.poi.hslf.model.HeadersFooters;
 import org.apache.poi.hslf.record.ColorSchemeAtom;
 import org.apache.poi.hslf.record.Comment2000;
 import org.apache.poi.hslf.record.EscherTextboxWrapper;
 import org.apache.poi.hslf.record.HeadersFootersContainer;
+import org.apache.poi.hslf.record.Record;
 import org.apache.poi.hslf.record.RecordContainer;
 import org.apache.poi.hslf.record.RecordTypes;
 import org.apache.poi.hslf.record.SSSlideInfoAtom;
@@ -87,8 +87,6 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
 	        if (_paragraphs.isEmpty()) {
 	            throw new HSLFException("No text records found for slide");
 	        }
-		} else {
-			// No text on the slide, must just be pictures
 		}
 
 		// Grab text from slide's PPDrawing
@@ -366,8 +364,9 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
      */
      @Override
     public HSLFBackground getBackground() {
-        if(getFollowMasterBackground()) {
+        if (getFollowMasterBackground()) {
-            return getMasterSheet().getBackground();
+            final HSLFMasterSheet ms = getMasterSheet();
+            return (ms == null) ? null : ms.getBackground();
         }
         return super.getBackground();
     }
@@ -377,64 +376,45 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
      */
     @Override
     public ColorSchemeAtom getColorScheme() {
-        if(getFollowMasterScheme()){
+        if (getFollowMasterScheme()) {
-            return getMasterSheet().getColorScheme();
+            final HSLFMasterSheet ms = getMasterSheet();
+            return (ms == null) ? null : ms.getColorScheme();
         }
         return super.getColorScheme();
     }
 
+    private static RecordContainer selectContainer(final RecordContainer root, final int index, final RecordTypes... path) {
+        if (root == null || index >= path.length) {
+            return root;
+        }
+        final RecordContainer newRoot = (RecordContainer) root.findFirstOfType(path[index].typeID);
+        return selectContainer(newRoot, index+1, path);
+    }
+
     /**
      * Get the comment(s) for this slide.
      * Note - for now, only works on PPT 2000 and
      *  PPT 2003 files. Doesn't work for PPT 97
      *  ones, as they do their comments oddly.
      */
-    public Comment[] getComments() {
+    public List<HSLFComment> getComments() {
+        final List<HSLFComment> comments = new ArrayList<>();
     	// If there are any, they're in
     	//  ProgTags -> ProgBinaryTag -> BinaryTagData
-    	RecordContainer progTags = (RecordContainer)
+        final RecordContainer binaryTags =
-    			getSheetContainer().findFirstOfType(
+                selectContainer(getSheetContainer(), 0,
-    						RecordTypes.ProgTags.typeID
+                        RecordTypes.ProgTags, RecordTypes.ProgBinaryTag, RecordTypes.BinaryTagData);
-    	);
-    	if(progTags != null) {
-    		RecordContainer progBinaryTag = (RecordContainer)
-    			progTags.findFirstOfType(
-    					RecordTypes.ProgBinaryTag.typeID
-    		);
-    		if(progBinaryTag != null) {
-    			RecordContainer binaryTags = (RecordContainer)
-    				progBinaryTag.findFirstOfType(
-    						RecordTypes.BinaryTagData.typeID
-    			);
-    			if(binaryTags != null) {
-    				// This is where they'll be
-    				int count = 0;
-    				for(int i=0; i<binaryTags.getChildRecords().length; i++) {
-    					if(binaryTags.getChildRecords()[i] instanceof Comment2000) {
-    						count++;
-    					}
-    				}
 
-    				// Now build
+        if (binaryTags != null) {
-    				Comment[] comments = new Comment[count];
+            for (final Record record : binaryTags.getChildRecords()) {
-    				count = 0;
+                if (record instanceof Comment2000) {
-    				for(int i=0; i<binaryTags.getChildRecords().length; i++) {
+                    comments.add(new HSLFComment((Comment2000)record));
-    					if(binaryTags.getChildRecords()[i] instanceof Comment2000) {
+                }
-    						comments[i] = new Comment(
-    								(Comment2000)binaryTags.getChildRecords()[i]
-    						);
-    						count++;
             }
         }
 
     	return comments;
     }
-    		}
-    	}
-
-    	// None found
-    	return new Comment[0];
-    }
 
     /**
      * Header / Footer settings for this slide.
@@ -478,9 +458,7 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
 	public boolean isHidden() {
 		SSSlideInfoAtom slideInfo =
 			(SSSlideInfoAtom)getSlideRecord().findFirstOfType(RecordTypes.SSSlideInfoAtom.typeID);
-		return (slideInfo == null)
+		return (slideInfo != null) && slideInfo.getEffectTransitionFlagByBit(SSSlideInfoAtom.HIDDEN_BIT);
-			? false
-			: slideInfo.getEffectTransitionFlagByBit(SSSlideInfoAtom.HIDDEN_BIT);
 	}
 
     @Override
@@ -505,12 +483,11 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
     }
 
     @Override
-    public boolean getDisplayPlaceholder(Placeholder placeholder) {
+    public boolean getDisplayPlaceholder(final Placeholder placeholder) {
-        HeadersFooters hf = getHeadersFooters();
+        final HeadersFooters hf = getHeadersFooters();
-        SlideLayoutType slt =  getSlideRecord().getSlideAtom().getSSlideLayoutAtom().getGeometryType();
+        final SlideLayoutType slt = getSlideRecord().getSlideAtom().getSSlideLayoutAtom().getGeometryType();
-        boolean isTitle =
+        final boolean isTitle =
             (slt == SlideLayoutType.TITLE_SLIDE || slt == SlideLayoutType.TITLE_ONLY || slt == SlideLayoutType.MASTER_TITLE);
-        if (hf != null) {
         switch (placeholder) {
         case DATETIME:
             return hf.isDateTimeVisible() && !isTitle;
@@ -521,9 +498,7 @@ public final class HSLFSlide extends HSLFSheet implements Slide<HSLFShape,HSLFTe
         case FOOTER:
             return hf.isFooterVisible() && !isTitle;
         default:
-                break;    
-            }
-        }
             return false;
         }
+    }
 }

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java

@@ -38,6 +38,7 @@ import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherOptRecord;
 import org.apache.poi.hpsf.ClassID;
 import org.apache.poi.hpsf.ClassIDPredefined;
+import org.apache.poi.hpsf.extractor.HPSFPropertiesExtractor;
 import org.apache.poi.hslf.exceptions.CorruptPowerPointFileException;
 import org.apache.poi.hslf.exceptions.HSLFException;
 import org.apache.poi.hslf.model.HeadersFooters;
@@ -1047,6 +1048,11 @@ public final class HSLFSlideShow implements SlideShow<HSLFShape,HSLFTextParagrap
 		return objectId;
 	}
 
+	@Override
+    public HPSFPropertiesExtractor getMetadataTextExtractor() {
+        return new HPSFPropertiesExtractor(getSlideShowImpl());
+    }
+	
 	protected int addToObjListAtom(RecordContainer exObj) {
 		ExObjList lst = getDocumentRecord().getExObjList(true);
 		ExObjListAtom objAtom = lst.getExObjListAtom();

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextShape.java

@@ -701,18 +701,13 @@ implements TextShape<HSLFShape,HSLFTextParagraph> {
 
     @Override
     public boolean isPlaceholder() {
-        OEPlaceholderAtom oep = getPlaceholderAtom();
+        return
-        if (oep != null) {
+            ((getPlaceholderAtom() != null) ||
-            return true;
-        }
-
             //special case for files saved in Office 2007
-        RoundTripHFPlaceholder12 hldr = getHFPlaceholderAtom();
+            (getHFPlaceholderAtom() != null)) &&
-        if (hldr != null) {
+            // check for metro shape of complex placeholder
-            return true;
+            (!new HSLFMetroShape<HSLFShape>(this).hasMetroBlob())
-        }
+        ;
-
-        return false;
     }
 
 

src/scratchpad/testcases/org/apache/poi/hslf/extractor/TestExtractor.java

@@ -33,9 +33,9 @@ import java.io.InputStream;
 import java.util.List;
 
 import org.apache.poi.POIDataSamples;
+import org.apache.poi.hslf.usermodel.HSLFObjectShape;
 import org.apache.poi.hslf.usermodel.HSLFSlideShow;
 import org.apache.poi.hslf.usermodel.HSLFSlideShowImpl;
-import org.apache.poi.hslf.usermodel.HSLFObjectShape;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hwpf.HWPFDocument;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
@@ -89,12 +89,12 @@ public final class TestExtractor {
     public void testReadSheetText() throws IOException {
         // Basic 2 page example
         PowerPointExtractor ppe = openExtractor("basic_test_ppt_file.ppt");
-        ensureTwoStringsTheSame(expectText, ppe.getText());
+        assertEquals(expectText, ppe.getText());
         ppe.close();
 
         // 1 page example with text boxes
         PowerPointExtractor ppe2 = openExtractor("with_textbox.ppt");
-        ensureTwoStringsTheSame(expectText2, ppe2.getText());
+        assertEquals(expectText2, ppe2.getText());
         ppe2.close();
     }
 
@@ -103,15 +103,15 @@ public final class TestExtractor {
         // Basic 2 page example
         PowerPointExtractor ppe = openExtractor("basic_test_ppt_file.ppt");
         String notesText = ppe.getNotes();
-        String expText = "These are the notes for page 1\nThese are the notes on page two, again lacking formatting\n";
+        String expText = "\nThese are the notes for page 1\n\nThese are the notes on page two, again lacking formatting\n";
-        ensureTwoStringsTheSame(expText, notesText);
+        assertEquals(expText, notesText);
         ppe.close();
 
         // Other one doesn't have notes
         PowerPointExtractor ppe2 = openExtractor("with_textbox.ppt");
         notesText = ppe2.getNotes();
         expText = "";
-        ensureTwoStringsTheSame(expText, notesText);
+        assertEquals(expText, notesText);
         ppe2.close();
     }
 
@@ -122,8 +122,8 @@ public final class TestExtractor {
                 "This is the title on page 2\nThis is page two\nIt has several blocks of text\nNone of them have formatting\n"
         };
         String[] ntText = new String[]{
-                "These are the notes for page 1\n",
+                "\nThese are the notes for page 1\n",
-                "These are the notes on page two, again lacking formatting\n"
+                "\nThese are the notes on page two, again lacking formatting\n"
         };
 
         PowerPointExtractor ppe = openExtractor("basic_test_ppt_file.ppt");
@@ -137,7 +137,7 @@ public final class TestExtractor {
 
         ppe.setSlidesByDefault(true);
         ppe.setNotesByDefault(true);
-        assertEquals(slText[0] + slText[1] + "\n" + ntText[0] + ntText[1], ppe.getText());
+        assertEquals(slText[0] + ntText[0] + slText[1] + ntText[1], ppe.getText());
         ppe.close();
     }
 
@@ -166,16 +166,6 @@ public final class TestExtractor {
         ppe.close();
     }
 
-    private void ensureTwoStringsTheSame(String exp, String act) {
-        assertEquals(exp.length(), act.length());
-        char[] expC = exp.toCharArray();
-        char[] actC = act.toCharArray();
-        for (int i = 0; i < expC.length; i++) {
-            assertEquals("Char " + i, expC[i], actC[i]);
-        }
-        assertEquals(exp, act);
-    }
-
     @Test
     public void testExtractFromEmbeded() throws IOException {
         InputStream is = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("excel_with_embeded.xls");
@@ -454,4 +444,38 @@ public final class TestExtractor {
         assertContains(text, "Prague");
         ppe.close();
     }
+
+    @Test
+    public void testExtractGroupedShapeText() throws Exception {
+        try (final PowerPointExtractor ppe = openExtractor("bug62092.ppt")) {
+            final String text = ppe.getText();
+
+            //this tests that we're ignoring text shapes at depth=0
+            //i.e. POI has already included them in the slide's getTextParagraphs()
+            assertContains(text, "Text box1");
+            assertEquals(1, countMatches(text,"Text box1"));
+
+
+            //the WordArt and text box count tests will fail
+            //if this content is available via getTextParagraphs() of the slide in POI
+            //i.e. when POI is fixed, these tests will fail, and
+            //we'll have to remove the workaround in HSLFExtractor's extractGroupText(...)
+            assertEquals(1, countMatches(text,"WordArt1"));
+            assertEquals(1, countMatches(text,"WordArt2"));
+            assertEquals(1, countMatches(text,"Ungrouped text box"));//should only be 1
+            assertContains(text, "Text box2");
+            assertContains(text, "Text box3");
+            assertContains(text, "Text box4");
+            assertContains(text, "Text box5");
+
+            //see below -- need to extract hyperlinks
+            assertContains(text, "tika");
+            assertContains(text, "MyTitle");
+
+        }
+    }
+
+    private static int countMatches(final String base, final String find) {
+        return base.split(find).length-1;
+    }
 }

src/scratchpad/testcases/org/apache/poi/hslf/record/TestRecordTypes.java

@@ -18,10 +18,10 @@
 package org.apache.poi.hslf.record;
 
 
-import static org.junit.Assert.assertEquals;
-
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
+
 /**
  * Tests that RecordTypes returns the right records and classes when asked
  */
@@ -42,20 +42,15 @@ public final class TestRecordTypes {
 
     @Test
 	public void testPPTClassLookups() {
-		assertEquals(Slide.class, RecordTypes.Slide.handlingClass);
-		assertEquals(TextCharsAtom.class, RecordTypes.TextCharsAtom.handlingClass);
-		assertEquals(TextBytesAtom.class, RecordTypes.TextBytesAtom.handlingClass);
-		assertEquals(SlideListWithText.class, RecordTypes.SlideListWithText.handlingClass);
-
 		// If this record is ever implemented, change to one that isn't!
 		// This is checking the "unhandled default" stuff works
-		assertEquals(UnknownRecordPlaceholder.class, RecordTypes.forTypeID(-10).handlingClass);
+		assertEquals(RecordTypes.UnknownRecordPlaceholder, RecordTypes.forTypeID(-10));
 	}
 
     @Test
     public void testEscherClassLookups() {
 		// Should all come back with null, as DDF handles them
-		assertEquals(null, RecordTypes.EscherDggContainer.handlingClass);
+		assertEquals(null, RecordTypes.EscherDggContainer.recordConstructor);
-		assertEquals(null, RecordTypes.EscherBStoreContainer.handlingClass);
+		assertEquals(null, RecordTypes.EscherBStoreContainer.recordConstructor);
 	}
 }