Use RPC for TextField cursor position and selection range updates

Change-Id: I48595a1d1a9a1620739d00a499d996026bd51000

7 files changed, 287 insertions, 94 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/textfield/TextFieldConnector.java b/client/src/main/java/com/vaadin/client/ui/textfield/TextFieldConnector.java
index aec7b0f3d5..2c1c64b633 100644
--- a/client/src/main/java/com/vaadin/client/ui/textfield/TextFieldConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/textfield/TextFieldConnector.java
@@ -17,7 +17,6 @@
 package com.vaadin.client.ui.textfield;
 
 import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.Timer;
 import com.vaadin.client.annotations.OnStateChange;
 import com.vaadin.client.event.InputEvent;
@@ -26,6 +25,7 @@ import com.vaadin.client.ui.ConnectorFocusAndBlurHandler;
 import com.vaadin.client.ui.VTextField;
 import com.vaadin.shared.ui.Connect;
 import com.vaadin.shared.ui.Connect.LoadStyle;
+import com.vaadin.shared.ui.textfield.AbstractTextFieldClientRpc;
 import com.vaadin.shared.ui.textfield.AbstractTextFieldServerRpc;
 import com.vaadin.shared.ui.textfield.TextFieldState;
 import com.vaadin.shared.ui.textfield.ValueChangeMode;
@@ -37,6 +37,35 @@ import com.vaadin.ui.TextField;
 @Connect(value = TextField.class, loadStyle = LoadStyle.EAGER)
 public class TextFieldConnector extends AbstractComponentConnector {
 
+    private class AbstractTextFieldClientRpcImpl
+            implements AbstractTextFieldClientRpc {
+        @Override
+        public void selectRange(int start, int length) {
+            int textLength = getWidget().getText().length();
+            start = restrictTo(start, 0, textLength - 1);
+            length = restrictTo(length, 0, textLength - start);
+            getWidget().setSelectionRange(start, length);
+        }
+
+        private int restrictTo(int value, int min, int max) {
+            if (value < min) {
+                value = min;
+            }
+            if (value > max) {
+                value = max;
+            }
+
+            return value;
+        }
+
+        @Override
+        public void selectAll() {
+            getWidget().selectAll();
+        }
+    }
+
+    private int lastSentCursorPosition = -1;
+
     private Timer valueChangeTrigger = new Timer() {
         @Override
         public void run() {
@@ -46,6 +75,8 @@ public class TextFieldConnector extends AbstractComponentConnector {
 
     @Override
     protected void init() {
+        registerRpc(AbstractTextFieldClientRpc.class,
+                new AbstractTextFieldClientRpcImpl());
         ConnectorFocusAndBlurHandler.addHandlers(this);
         getWidget().addChangeHandler(event -> sendValueChange());
         getWidget().addDomHandler(event -> {
@@ -102,33 +133,10 @@ public class TextFieldConnector extends AbstractComponentConnector {
         getWidget().setReadOnly(getState().readOnly);
     }
 
-    @OnStateChange({ "selectionStart", "selectionLength" })
-    private void updateSelection() {
-        if (getState().selectionStart != -1) {
-            Scheduler.get().scheduleDeferred(new Command() {
-                @Override
-                public void execute() {
-                    getWidget().setSelectionRange(getState().selectionStart,
-                            getState().selectionLength);
-                }
-            });
-        }
-    }
-
-    @OnStateChange("cursorPosition")
-    private void updateCursorPosition() {
-        Scheduler.get().scheduleDeferred(new Command() {
-            @Override
-            public void execute() {
-                getWidget().setCursorPos(getState().cursorPosition);
-            }
-        });
-    }
-
     private boolean hasStateChanged() {
         boolean textChanged = !getWidget().getValue().equals(getState().text);
         boolean cursorPosChanged = getWidget()
-                .getCursorPos() != getState().cursorPosition;
+                .getCursorPos() != lastSentCursorPosition;
         return textChanged || cursorPosChanged;
     }
 
@@ -136,9 +144,9 @@ public class TextFieldConnector extends AbstractComponentConnector {
         if (!hasStateChanged()) {
             return;
         }
+        lastSentCursorPosition = getWidget().getCursorPos();
         getRpcProxy(AbstractTextFieldServerRpc.class)
-                .setText(getWidget().getValue(), getWidget().getCursorPos());
+                .setText(getWidget().getValue(), lastSentCursorPosition);
         getState().text = getWidget().getValue();
-        getState().cursorPosition = getWidget().getCursorPos();
     }
 }
diff --git a/server/src/main/java/com/vaadin/ui/AbstractTextField.java b/server/src/main/java/com/vaadin/ui/AbstractTextField.java
index f6e31d6da3..8043766a11 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractTextField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractTextField.java
@@ -26,6 +26,7 @@ import com.vaadin.event.FieldEvents.BlurListener;
 import com.vaadin.event.FieldEvents.FocusEvent;
 import com.vaadin.event.FieldEvents.FocusListener;
 import com.vaadin.shared.Registration;
+import com.vaadin.shared.ui.textfield.AbstractTextFieldClientRpc;
 import com.vaadin.shared.ui.textfield.AbstractTextFieldServerRpc;
 import com.vaadin.shared.ui.textfield.TextFieldState;
 import com.vaadin.shared.ui.textfield.ValueChangeMode;
@@ -56,13 +57,16 @@ public abstract class AbstractTextField extends AbstractField<String> {
         public void setText(String text, int cursorPosition) {
             getUI().getConnectorTracker().getDiffState(AbstractTextField.this)
                     .put("text", text);
-            getUI().getConnectorTracker().getDiffState(AbstractTextField.this)
-                    .put("cursorPosition", cursorPosition);
-            getState(false).cursorPosition = cursorPosition;
+            lastKnownCursorPosition = cursorPosition;
             setValue(text, true);
         }
     }
 
+    private int lastKnownCursorPosition = -1;
+
+    /**
+     * Creates a new instance.
+     */
     protected AbstractTextField() {
         registerRpc(new TextFieldServerRpcImpl());
     }
@@ -125,24 +129,27 @@ public abstract class AbstractTextField extends AbstractField<String> {
 
     /**
      * Selects all text in the field.
+     * <p>
+     * As a side effect the field will become focused.
      */
     public void selectAll() {
-        setSelection(0, getValue().length());
+        getRpcProxy(AbstractTextFieldClientRpc.class).selectAll();
+        focus();
     }
 
     /**
      * Sets the range of text to be selected.
-     *
+     * <p>
      * As a side effect the field will become focused.
      *
-     * @param pos
+     * @param start
      *            the position of the first character to be selected
      * @param length
      *            the number of characters to be selected
      */
     public void setSelection(int start, int length) {
-        getState().selectionStart = start;
-        getState().selectionLength = length;
+        getRpcProxy(AbstractTextFieldClientRpc.class).selectRange(start,
+                length);
         focus();
     }
 
@@ -154,8 +161,7 @@ public abstract class AbstractTextField extends AbstractField<String> {
      *            the position for the cursor
      */
     public void setCursorPosition(int pos) {
-        getState().cursorPosition = pos;
-        focus();
+        setSelection(pos, 0);
     }
 
     /**
@@ -163,7 +169,7 @@ public abstract class AbstractTextField extends AbstractField<String> {
      *
      */
     public int getCursorPosition() {
-        return getState(false).cursorPosition;
+        return lastKnownCursorPosition;
     }
 
     /**
diff --git a/shared/src/main/java/com/vaadin/shared/ui/textfield/AbstractTextFieldClientRpc.java b/shared/src/main/java/com/vaadin/shared/ui/textfield/AbstractTextFieldClientRpc.java
new file mode 100644
index 0000000000..171707d848
--- /dev/null
+++ b/shared/src/main/java/com/vaadin/shared/ui/textfield/AbstractTextFieldClientRpc.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2000-2016 Vaadin Ltd.
+ *
+ * Licensed 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 com.vaadin.shared.ui.textfield;
+
+import com.vaadin.shared.communication.ClientRpc;
+
+/**
+ * Server to client RPC interface for AbstractTextField.
+ *
+ */
+public interface AbstractTextFieldClientRpc extends ClientRpc {
+    /**
+     * Selects the given range in the field.
+     *
+     * @param start
+     *            the start of the range
+     * @param length
+     *            the length to select
+     */
+    void selectRange(int start, int length);
+
+    /**
+     * Selects everything in the field.
+     */
+    void selectAll();
+}
diff --git a/shared/src/main/java/com/vaadin/shared/ui/textfield/TextFieldState.java b/shared/src/main/java/com/vaadin/shared/ui/textfield/TextFieldState.java
index a61a2897f7..f30af9ec78 100644
--- a/shared/src/main/java/com/vaadin/shared/ui/textfield/TextFieldState.java
+++ b/shared/src/main/java/com/vaadin/shared/ui/textfield/TextFieldState.java
@@ -51,15 +51,6 @@ public class TextFieldState extends AbstractFieldState {
     public String text = "";
 
     @NoLayout
-    public int selectionStart = -1;
-
-    @NoLayout
-    public int selectionLength = 0;
-
-    @NoLayout
-    public int cursorPosition = 0;
-
-    @NoLayout
     public ValueChangeMode valueChangeMode = ValueChangeMode.LAZY;
 
     @NoLayout
diff --git a/uitest/src/main/java/com/vaadin/tests/components/textfield/SelectionAndCursorPosition.java b/uitest/src/main/java/com/vaadin/tests/components/textfield/SelectionAndCursorPosition.java
index 42f648cfc2..61ae5e535d 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/textfield/SelectionAndCursorPosition.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/textfield/SelectionAndCursorPosition.java
@@ -1,10 +1,9 @@
 package com.vaadin.tests.components.textfield;
 
-import com.vaadin.tests.components.TestBase;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
 import com.vaadin.ui.AbstractTextField;
 import com.vaadin.ui.Button;
-import com.vaadin.ui.Button.ClickEvent;
-import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.CheckBox;
 import com.vaadin.ui.FormLayout;
 import com.vaadin.ui.HorizontalLayout;
@@ -12,14 +11,24 @@ import com.vaadin.ui.Panel;
 import com.vaadin.ui.TextField;
 import com.vaadin.v7.ui.TextArea;
 
-public class SelectionAndCursorPosition extends TestBase {
+public class SelectionAndCursorPosition extends AbstractTestUI {
+
+    static final String DEFAULT_TEXT = "So we have some text to select";
+    static final String TEXTFIELD_ID = "tf";
+    static final String TEXTAREA_ID = "ta";
+    static final String SELECT_ALL_ID = "selectAll";
+    static final String RANGE_START_ID = "rS";
+    static final String RANGE_LENGTH_ID = "rL";
+    static final String CURSOR_POS_ID = "cp";
+    static final String RANGE_SET_BUTTON_ID = "setSelection";
+    static final String CURSOR_POS_SET_ID = "cps";
 
     TextField textField = createTextField();
     TextArea textArea = createTextArea();
     AbstractTextField activeComponent = textField;
 
     @Override
-    protected void setup() {
+    protected void setup(VaadinRequest request) {
         FormLayout fl = new FormLayout();
         Panel panel = new Panel(fl);
         panel.setCaption("Hackers panel");
@@ -42,49 +51,44 @@ public class SelectionAndCursorPosition extends TestBase {
         // });
         fl.addComponent(ml);
 
-        Button b = new Button("Select all ( selectAll() )");
-        b.addListener(new ClickListener() {
-            @Override
-            public void buttonClick(ClickEvent event) {
-                activeComponent.selectAll();
-            }
-        });
-        fl.addComponent(b);
+        Button selectAll = new Button("Select all ( selectAll() )");
+        selectAll.setId(SELECT_ALL_ID);
+        selectAll.addClickListener(event -> activeComponent.selectAll());
+        fl.addComponent(selectAll);
 
         HorizontalLayout selectRange = new HorizontalLayout();
         selectRange.setCaption(
                 "Select range of text ( setSelectionRange(int start, int lengt) )");
         final TextField start = new TextField("From:");
+        start.setId(RANGE_START_ID);
         final TextField length = new TextField("Selection length:");
-        b = new Button("select");
-        b.addListener(new ClickListener() {
-            @Override
-            public void buttonClick(ClickEvent event) {
-                int startPos = Integer.parseInt(start.getValue());
-                int lenght = Integer.parseInt(length.getValue());
-
-                activeComponent.setSelection(startPos, lenght);
-            }
+        length.setId(RANGE_LENGTH_ID);
+        Button select = new Button("select");
+        select.setId(RANGE_SET_BUTTON_ID);
+        select.addClickListener(event -> {
+            int startPos = Integer.parseInt(start.getValue());
+            int lenght = Integer.parseInt(length.getValue());
+
+            activeComponent.setSelection(startPos, lenght);
         });
 
         selectRange.addComponent(start);
         selectRange.addComponent(length);
-        selectRange.addComponent(b);
+        selectRange.addComponent(select);
         fl.addComponent(selectRange);
 
         HorizontalLayout setCursorPosition = new HorizontalLayout();
         final TextField pos = new TextField("Position:");
-        b = new Button("set");
-        b.addListener(new ClickListener() {
-            @Override
-            public void buttonClick(ClickEvent event) {
-                int startPos = Integer.parseInt(pos.getValue());
-                activeComponent.setCursorPosition(startPos);
-            }
+        pos.setId(CURSOR_POS_ID);
+        Button setCursorButton = new Button("set");
+        setCursorButton.setId(CURSOR_POS_SET_ID);
+        setCursorButton.addClickListener(event -> {
+            int startPos = Integer.parseInt(pos.getValue());
+            activeComponent.setCursorPosition(startPos);
         });
 
         setCursorPosition.addComponent(pos);
-        setCursorPosition.addComponent(b);
+        setCursorPosition.addComponent(setCursorButton);
         setCursorPosition.setCaption(
                 "Set cursor position ( setCursorPosition(int pos) )");
         fl.addComponent(setCursorPosition);
@@ -95,29 +99,29 @@ public class SelectionAndCursorPosition extends TestBase {
     }
 
     private static TextField createTextField() {
-        TextField tf = new TextField();
-        tf.setCaption("Text field");
-        tf.setValue("So we have some text to select");
-        tf.setWidth("400px");
+        TextField textField = new TextField();
+        textField.setId(TEXTFIELD_ID);
+        textField.setCaption("Text field");
+        textField.setValue(DEFAULT_TEXT);
+        textField.setWidth("400px");
 
-        return tf;
+        return textField;
     }
 
     private static TextArea createTextArea() {
-        TextArea ta = new TextArea();
-        ta.setCaption("Text area");
-        ta.setValue("So we have some text to select");
-        ta.setWidth("400px");
-        ta.setHeight("50px");
-
-        return ta;
+        TextArea textArea = new TextArea();
+        textArea.setId(TEXTAREA_ID);
+        textArea.setCaption("Text area");
+        textArea.setValue(DEFAULT_TEXT);
+        textArea.setWidth("400px");
+        textArea.setHeight("50px");
+
+        return textArea;
     }
 
     @Override
-    protected String getDescription() {
-        return "For usability reasons it is often essential that developer "
-                + "can hint how to select the text in the "
-                + "field or where to set the cursor position.";
+    protected String getTestDescription() {
+        return "Tests that setSelectionRange and setCursorPosition works for a TextField";
     }
 
     @Override
diff --git a/uitest/src/test/java/com/vaadin/tests/components/textfield/Selection.java b/uitest/src/test/java/com/vaadin/tests/components/textfield/Selection.java
new file mode 100644
index 0000000000..7988339344
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/textfield/Selection.java
@@ -0,0 +1,54 @@
+package com.vaadin.tests.components.textfield;
+
+import java.util.List;
+
+public class Selection {
+    private int start, length;
+
+    public Selection(List<Long> range) {
+        start = range.get(0).intValue();
+        length = range.get(1).intValue() - start;
+    }
+
+    public Selection(int start, int length) {
+        super();
+        this.start = start;
+        this.length = length;
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + length;
+        result = prime * result + start;
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null) {
+            return false;
+        }
+        if (getClass() != obj.getClass()) {
+            return false;
+        }
+        Selection other = (Selection) obj;
+        if (length != other.length) {
+            return false;
+        }
+        if (start != other.start) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public String toString() {
+        return "Selection [start=" + start + ", length=" + length + "]";
+    }
+
+}
\ No newline at end of file
diff --git a/uitest/src/test/java/com/vaadin/tests/components/textfield/SelectionAndCursorPositionTest.java b/uitest/src/test/java/com/vaadin/tests/components/textfield/SelectionAndCursorPositionTest.java
new file mode 100644
index 0000000000..8b540b1095
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/textfield/SelectionAndCursorPositionTest.java
@@ -0,0 +1,91 @@
+package com.vaadin.tests.components.textfield;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class SelectionAndCursorPositionTest extends SingleBrowserTest {
+
+    private static final int DEFAULT_TEXT_LENGTH = SelectionAndCursorPosition.DEFAULT_TEXT
+            .length();
+    private WebElement textField;
+
+    @Test
+    public void testSelection() {
+        openTestURL();
+        textField = findElement(By.id(SelectionAndCursorPosition.TEXTFIELD_ID));
+
+        // Select all
+        getSelectAll().click();
+        assertSelection(0, DEFAULT_TEXT_LENGTH);
+
+        // Select range
+        setSelectionRange(10, 5);
+        assertSelection(10, 5);
+
+        // Test for index out of bounds
+        setSelectionRange(0, DEFAULT_TEXT_LENGTH);
+        assertSelection(0, DEFAULT_TEXT_LENGTH);
+        setSelectionRange(0, DEFAULT_TEXT_LENGTH + 1);
+        assertSelection(0, DEFAULT_TEXT_LENGTH);
+        setSelectionRange(1, DEFAULT_TEXT_LENGTH);
+        assertSelection(1, DEFAULT_TEXT_LENGTH - 1);
+        setSelectionRange(DEFAULT_TEXT_LENGTH - 1, 2);
+        assertSelection(DEFAULT_TEXT_LENGTH - 1, 1);
+
+        // Cursor position
+        setCursorPosition(0);
+        assertCursorPosition(0);
+
+    }
+
+    private void assertCursorPosition(int i) {
+        assertSelection(i, 0);
+    }
+
+    private void setCursorPosition(int i) {
+        $(TextFieldElement.class).id(SelectionAndCursorPosition.CURSOR_POS_ID)
+                .setValue(String.valueOf(i));
+        $(ButtonElement.class).id(SelectionAndCursorPosition.CURSOR_POS_SET_ID)
+                .click();
+
+    }
+
+    private void setSelectionRange(int start, int length) {
+        $(TextFieldElement.class).id(SelectionAndCursorPosition.RANGE_START_ID)
+                .setValue(String.valueOf(start));
+        $(TextFieldElement.class).id(SelectionAndCursorPosition.RANGE_LENGTH_ID)
+                .setValue(String.valueOf(length));
+        $(ButtonElement.class)
+                .id(SelectionAndCursorPosition.RANGE_SET_BUTTON_ID).click();
+    }
+
+    private void assertSelection(int start, int length) {
+        Assert.assertEquals(new Selection(start, length),
+                getSelection(textField));
+    }
+
+    private void clearSelection() {
+        setSelectionRange(0, 0);
+
+    }
+
+    private WebElement getSelectAll() {
+        return findElement(By.id(SelectionAndCursorPosition.SELECT_ALL_ID));
+    }
+
+    private Selection getSelection(WebElement textField) {
+        @SuppressWarnings("unchecked")
+        List<Long> range = (List<Long>) executeScript(
+                "return [arguments[0].selectionStart,arguments[0].selectionEnd]",
+                textField);
+        return new Selection(range);
+    }
+}