Merge pull request #595 from afester/Issue594

Issue #594: Properly calculate the background and underline shapes fo…

richtextfx/src/integrationTest/java/org/fxmisc/richtext/SceneGraphTests.java

@@ -0,0 +1,82 @@
+package org.fxmisc.richtext;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.fxmisc.flowless.Cell;
+import org.fxmisc.flowless.VirtualFlow;
+import org.fxmisc.richtext.UnderlinePath;
+
+import javafx.scene.Node;
+import javafx.scene.layout.Region;
+import javafx.scene.shape.Path;
+import javafx.scene.text.Text;
+import javafx.scene.text.TextFlow;
+
+/**
+ * Contains inspection methods to analyze the scene graph which has been rendered by RichTextFX.
+ * TestFX tests should subclass this if it needs to run tests on a simple area and needs to inspect
+ * whether the scene graph has been properly created.
+ */
+public abstract class SceneGraphTests extends InlineCssTextAreaAppTest {
+
+    /**
+     * @param index The index of the desired paragraph box
+     * @return The paragraph box for the paragraph at the specified index
+     */
+    protected Region getParagraphBox(int index) {
+        @SuppressWarnings("unchecked")
+        VirtualFlow<String, Cell<String, Node>> flow = (VirtualFlow<String, Cell<String, Node>>) area.getChildrenUnmodifiable().get(index);
+        Cell<String, Node> gsa = flow.getCell(0);
+
+        // get the ParagraphBox (protected subclass of Region) 
+        return (Region) gsa.getNode();
+    }
+
+
+    /**
+     * @param index The index of the desired paragraph box
+     * @return The ParagraphText (protected subclass of TextFlow) for the paragraph at the specified index
+     */
+    protected TextFlow getParagraphText(int index) {
+        // get the ParagraphBox (protected subclass of Region) 
+        Region paragraphBox = getParagraphBox(index);
+
+        // get the ParagraphText (protected subclass of TextFlow)
+        TextFlow tf = (TextFlow) paragraphBox.getChildrenUnmodifiable().stream().filter(n -> n instanceof TextFlow)
+                                 .findFirst().orElse(null);
+        assertNotNull("No TextFlow node found in rich text area", tf);
+
+        return tf;
+    }
+
+
+    /**
+     * @param index The index of the desired paragraph box
+     * @return A list of text nodes which render the text in the ParagraphBox 
+     *         specified by the given index.
+     */
+    protected List<Text> getTextNodes(int index) {
+        TextFlow tf = getParagraphText(index);
+
+        List<Text> result = new ArrayList<>();
+        tf.getChildrenUnmodifiable().filtered(n -> n instanceof Text).forEach(n -> result.add((Text) n));
+        return result;
+    }
+
+
+    /**
+     * @param index The index of the desired paragraph box
+     * @return A list of nodes which render the underlines for the text in the ParagraphBox 
+     *         specified by the given index.
+     */
+    protected List<Path> getUnderlinePaths(int index) {
+        TextFlow tf = getParagraphText(index);
+
+        List<Path> result = new ArrayList<>();
+        tf.getChildrenUnmodifiable().filtered(n -> n instanceof UnderlinePath).forEach(n -> result.add((Path) n));
+        return result;
+    }
+}

richtextfx/src/integrationTest/java/org/fxmisc/richtext/keyboard/PageUpDownTests.java

@@ -3,7 +3,6 @@
 import javafx.geometry.Bounds;
 import javafx.stage.Stage;
 import org.fxmisc.richtext.InlineCssTextAreaAppTest;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static javafx.scene.input.KeyCode.PAGE_DOWN;

richtextfx/src/integrationTest/java/org/fxmisc/richtext/style/StylingTests.java

@@ -0,0 +1,100 @@
+package org.fxmisc.richtext.style;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.List;
+
+import org.fxmisc.richtext.SceneGraphTests;
+import org.junit.Test;
+
+import javafx.scene.shape.Path;
+import javafx.scene.text.Text;
+
+
+public class StylingTests extends SceneGraphTests {
+
+    private final static String HELLO = "Hello ";
+    private final static String WORLD = "World";
+    private final static String AND_ALSO_THE = " and also the ";
+    private final static String SUN = "Sun";
+    private final static String AND_MOON = " and Moon";
+
+    @Test
+    public void simpleStyling() {
+        // setup
+        interact(() -> {
+            area.replaceText(HELLO + WORLD + AND_MOON);
+        });
+
+        // expected: one text node which contains the complete text
+        List<Text> textNodes = getTextNodes(0);
+        assertEquals(1, textNodes.size());
+
+        interact(() -> {
+            area.setStyle(HELLO.length(), HELLO.length() + WORLD.length(), "-fx-font-weight: bold;");
+        });
+
+        // expected: three text nodes
+        textNodes = getTextNodes(0);
+        assertEquals(3, textNodes.size());
+
+        Text first = textNodes.get(0);
+        assertEquals("Hello ", first.getText());
+        assertEquals("Regular", first.getFont().getStyle());
+
+        Text second = textNodes.get(1);
+        assertEquals("World", second.getText());
+        assertEquals("Bold", second.getFont().getStyle());
+
+        Text third = textNodes.get(2);
+        assertEquals(" and Moon", third.getText());
+        assertEquals("Regular", third.getFont().getStyle());
+    }
+
+
+    @Test
+    public void underlineStyling() {
+
+        final String underlineStyle = "-rtfx-underline-color: red; -rtfx-underline-dash-array: 2 2; -rtfx-underline-width: 1; -rtfx-underline-cap: butt;";
+
+        // setup
+        interact(() -> {
+            area.replaceText(HELLO + WORLD + AND_ALSO_THE + SUN + AND_MOON);
+        });
+
+        // expected: one text node which contains the complete text
+        List<Text> textNodes = getTextNodes(0);
+        assertEquals(1, textNodes.size());
+        assertEquals(HELLO + WORLD + AND_ALSO_THE + SUN + AND_MOON, 
+                     textNodes.get(0).getText());
+
+        interact(() -> {
+            final int start1 = HELLO.length();
+            final int end1 = start1 + WORLD.length();
+            area.setStyle(start1, end1, underlineStyle);
+
+            final int start2 = end1 + AND_ALSO_THE.length();
+            final int end2 = start2 + SUN.length();
+            area.setStyle(start2, end2, underlineStyle);
+        });
+
+        // expected: five text nodes
+        textNodes = getTextNodes(0);
+        assertEquals(5, textNodes.size());
+
+        Text first = textNodes.get(0);
+        assertEquals(HELLO, first.getText());
+        Text second = textNodes.get(1);
+        assertEquals(WORLD, second.getText());
+        Text third = textNodes.get(2);
+        assertEquals(AND_ALSO_THE, third.getText());
+        Text fourth = textNodes.get(3);
+        assertEquals(SUN, fourth.getText());
+        Text fifth = textNodes.get(4);
+        assertEquals(AND_MOON, fifth.getText());
+
+        // determine the underline paths - need to be two of them!
+        List<Path> underlineNodes = getUnderlinePaths(0);
+        assertEquals(2, underlineNodes.size());
+    }
+}

richtextfx/src/main/java/org/fxmisc/richtext/BackgroundPath.java

@@ -0,0 +1,10 @@
+package org.fxmisc.richtext;
+
+import javafx.scene.shape.Path;
+
+/**
+ * A path which describes a background shape in the Scene graph.
+ * 
+ */
+public class BackgroundPath extends Path {
+}

richtextfx/src/main/java/org/fxmisc/richtext/BorderPath.java

@@ -0,0 +1,10 @@
+package org.fxmisc.richtext;
+
+import javafx.scene.shape.Path;
+
+/**
+ * A path which describes a border in the Scene graph.
+ * 
+ */
+public class BorderPath extends Path {
+}

richtextfx/src/main/java/org/fxmisc/richtext/CaretPath.java

@@ -0,0 +1,10 @@
+package org.fxmisc.richtext;
+
+import javafx.scene.shape.Path;
+
+/**
+ * A path which describes a caret shape in the Scene graph.
+ * 
+ */
+public class CaretPath extends Path {
+}

richtextfx/src/main/java/org/fxmisc/richtext/ParagraphText.java

@@ -56,8 +56,8 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
 
     private final Paragraph<PS, SEG, S> paragraph;
 
-    private final Path caretShape = new Path();
-    private final Path selectionShape = new Path();
+    private final Path caretShape = new CaretPath();
+    private final Path selectionShape = new SelectionPath();
 
     private final CustomCssShapeHelper<Paint> backgroundShapeHelper;
     private final CustomCssShapeHelper<BorderAttributes> borderShapeHelper;
@@ -115,18 +115,33 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
         par.getStyledSegments().stream().map(nodeFactory).forEach(getChildren()::add);
 
         // set up custom css shape helpers
-        Supplier<Path> createShape = () -> {
-            Path shape = new Path();
+        Supplier<Path> createBackgroundShape = () -> {
+            Path shape = new BackgroundPath();
             shape.setManaged(false);
             shape.layoutXProperty().bind(leftInset);
             shape.layoutYProperty().bind(topInset);
             return shape;
         };
+        Supplier<Path> createBorderShape = () -> {
+            Path shape = new BorderPath();
+            shape.setManaged(false);
+            shape.layoutXProperty().bind(leftInset);
+            shape.layoutYProperty().bind(topInset);
+            return shape;
+        };
+        Supplier<Path> createUnderlineShape = () -> {
+            Path shape = new UnderlinePath();
+            shape.setManaged(false);
+            shape.layoutXProperty().bind(leftInset);
+            shape.layoutYProperty().bind(topInset);
+            return shape;
+        };
+
         Consumer<Collection<Path>> clearUnusedShapes = paths -> getChildren().removeAll(paths);
         Consumer<Path> addToBackground = path -> getChildren().add(0, path);
         Consumer<Path> addToForeground = path -> getChildren().add(path);
         backgroundShapeHelper = new CustomCssShapeHelper<>(
-                createShape,
+                createBackgroundShape,
                 (backgroundShape, tuple) -> {
                     backgroundShape.setStrokeWidth(0);
                     backgroundShape.setFill(tuple._1);
@@ -136,7 +151,7 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
                 clearUnusedShapes
         );
         borderShapeHelper = new CustomCssShapeHelper<>(
-                createShape,
+                createBorderShape,
                 (borderShape, tuple) -> {
                     BorderAttributes attributes = tuple._1;
                     borderShape.setStrokeWidth(attributes.width);
@@ -153,7 +168,7 @@ public ObjectProperty<Paint> highlightTextFillProperty() {
                 clearUnusedShapes
         );
         underlineShapeHelper = new CustomCssShapeHelper<>(
-                createShape,
+                createUnderlineShape,
                 (underlineShape, tuple) -> {
                     UnderlineAttributes attributes = tuple._1;
                     underlineShape.setStroke(attributes.color);
@@ -389,7 +404,12 @@ private void updateSharedShapeRange(T value, int start, int end) {
                 int lastIndex = ranges.size() - 1;
                 Tuple2<T, IndexRange> lastShapeValueRange = ranges.get(lastIndex);
                 T lastShapeValue = lastShapeValueRange._1;
-                if (lastShapeValue.equals(value)) {
+
+                // calculate smallest possible position which is consecutive to the given start position
+                final int prevEndNext = lastShapeValueRange.get2().getEnd() + 1;  
+                if (start <= prevEndNext &&         // Consecutive? 
+                    lastShapeValue.equals(value)) { // Same style?
+
                     IndexRange lastRange = lastShapeValueRange._2;
                     IndexRange extendedRange = new IndexRange(lastRange.getStart(), end);
                     ranges.set(lastIndex, Tuples.t(lastShapeValue, extendedRange));

richtextfx/src/main/java/org/fxmisc/richtext/SelectionPath.java

@@ -0,0 +1,10 @@
+package org.fxmisc.richtext;
+
+import javafx.scene.shape.Path;
+
+/**
+ * A path which describes a selection shape in the Scene graph.
+ * 
+ */
+public class SelectionPath extends Path {
+}

richtextfx/src/main/java/org/fxmisc/richtext/UnderlinePath.java

@@ -0,0 +1,10 @@
+package org.fxmisc.richtext;
+
+import javafx.scene.shape.Path;
+
+/**
+ * A path which describes an underline in the Scene graph.
+ * 
+ */
+public class UnderlinePath extends Path {
+}