From 76b42e680f0f409f253a3aa8499a18fd09329957 Mon Sep 17 00:00:00 2001 From: John Ahlroos Date: Thu, 10 Sep 2015 14:09:32 +0300 Subject: [PATCH] Parse readonly values for AbstractFields correctly (#18850) Change-Id: I1128ec0bda9b0fdad721f084c1e78cfe78e984f2 --- server/src/com/vaadin/ui/AbstractField.java | 32 ++++++++++++--- server/src/com/vaadin/ui/AbstractSelect.java | 31 +++++++++----- server/src/com/vaadin/ui/CheckBox.java | 5 ++- server/src/com/vaadin/ui/DateField.java | 2 +- server/src/com/vaadin/ui/PasswordField.java | 2 +- server/src/com/vaadin/ui/ProgressBar.java | 5 ++- server/src/com/vaadin/ui/RichTextArea.java | 2 +- server/src/com/vaadin/ui/Slider.java | 7 ++-- server/src/com/vaadin/ui/TextArea.java | 3 +- server/src/com/vaadin/ui/TextField.java | 5 ++- .../checkbox/CheckboxDeclarativeTest.java | 10 +++++ .../combobox/ComboBoxDeclarativeTest.java | 16 ++++++++ .../datefield/DateFieldDeclarativeTest.java | 12 ++++++ .../NativeSelectDeclarativeTest.java | 16 ++++++++ .../PasswordFieldDeclarativeTest.java | 40 +++++++++++++++++++ .../ProgressBarDeclarativeTest.java | 14 ++++++- .../RichTextAreaDeclarativeTest.java | 11 +++++ .../slider/SliderDeclarativeTest.java | 15 +++++++ .../textarea/TextAreaDeclarativeTest.java | 11 +++++ .../textfield/TextFieldDeclarativeTest.java | 10 +++++ 20 files changed, 219 insertions(+), 30 deletions(-) create mode 100644 server/tests/src/com/vaadin/tests/server/component/passwordfield/PasswordFieldDeclarativeTest.java diff --git a/server/src/com/vaadin/ui/AbstractField.java b/server/src/com/vaadin/ui/AbstractField.java index cf14d1cb96..e69322b1cc 100644 --- a/server/src/com/vaadin/ui/AbstractField.java +++ b/server/src/com/vaadin/ui/AbstractField.java @@ -264,7 +264,9 @@ public abstract class AbstractField extends AbstractComponent implements committingValueToDataSource = false; } } else { - /* An invalid value and we don't allow them, throw the exception */ + /* + * An invalid value and we don't allow them, throw the exception + */ validate(); } } @@ -459,15 +461,35 @@ public abstract class AbstractField extends AbstractComponent implements * @param repaintIsNotNeeded * True iff caller is sure that repaint is not needed. * @throws Property.ReadOnlyException + * @throws Converter.ConversionException + * @throws InvalidValueException + */ + protected void setValue(T newFieldValue, boolean repaintIsNotNeeded) { + setValue(newFieldValue, repaintIsNotNeeded, false); + } + + /** + * Sets the value of the field. + * + * @since 7.5.7 + * @param newFieldValue + * the New value of the field. + * @param repaintIsNotNeeded + * True iff caller is sure that repaint is not needed. + * @param ignoreReadOnly + * True iff if the read-only check should be ignored + * @throws Property.ReadOnlyException + * @throws Converter.ConversionException + * @throws InvalidValueException */ - protected void setValue(T newFieldValue, boolean repaintIsNotNeeded) - throws Property.ReadOnlyException, Converter.ConversionException, - InvalidValueException { + protected void setValue(T newFieldValue, boolean repaintIsNotNeeded, + boolean ignoreReadOnly) throws Property.ReadOnlyException, + Converter.ConversionException, InvalidValueException { if (!SharedUtil.equals(newFieldValue, getInternalValue())) { // Read only fields can not be changed - if (isReadOnly()) { + if (!ignoreReadOnly && isReadOnly()) { throw new Property.ReadOnlyException(); } try { diff --git a/server/src/com/vaadin/ui/AbstractSelect.java b/server/src/com/vaadin/ui/AbstractSelect.java index bbd486a8f9..ee4cf87ed3 100644 --- a/server/src/com/vaadin/ui/AbstractSelect.java +++ b/server/src/com/vaadin/ui/AbstractSelect.java @@ -34,8 +34,10 @@ import org.jsoup.nodes.Element; import com.vaadin.data.Container; import com.vaadin.data.Item; import com.vaadin.data.Property; +import com.vaadin.data.Validator.InvalidValueException; import com.vaadin.data.util.IndexedContainer; import com.vaadin.data.util.converter.Converter; +import com.vaadin.data.util.converter.Converter.ConversionException; import com.vaadin.data.util.converter.ConverterUtil; import com.vaadin.event.DataBoundTransferable; import com.vaadin.event.Transferable; @@ -697,26 +699,33 @@ public abstract class AbstractSelect extends AbstractField implements * multiselect mode all collections of id:s can be assigned. *

* + * @since 7.5.7 * @param newValue * the New selected item or collection of selected items. * @param repaintIsNotNeeded * True if caller is sure that repaint is not needed. + * @param ignoreReadOnly + * True if read-only check should be omitted. * @see com.vaadin.ui.AbstractField#setValue(java.lang.Object, * java.lang.Boolean) */ @Override - protected void setValue(Object newValue, boolean repaintIsNotNeeded) - throws Property.ReadOnlyException { - + protected void setValue(Object newFieldValue, boolean repaintIsNotNeeded, + boolean ignoreReadOnly) + throws com.vaadin.data.Property.ReadOnlyException, + ConversionException, InvalidValueException { if (isMultiSelect()) { - if (newValue == null) { - super.setValue(new LinkedHashSet(), repaintIsNotNeeded); - } else if (Collection.class.isAssignableFrom(newValue.getClass())) { + if (newFieldValue == null) { + super.setValue(new LinkedHashSet(), repaintIsNotNeeded, + ignoreReadOnly); + } else if (Collection.class.isAssignableFrom(newFieldValue + .getClass())) { super.setValue(new LinkedHashSet( - (Collection) newValue), repaintIsNotNeeded); + (Collection) newFieldValue), repaintIsNotNeeded, + ignoreReadOnly); } - } else if (newValue == null || items.containsId(newValue)) { - super.setValue(newValue, repaintIsNotNeeded); + } else if (newFieldValue == null || items.containsId(newFieldValue)) { + super.setValue(newFieldValue, repaintIsNotNeeded, ignoreReadOnly); } } @@ -2196,9 +2205,9 @@ public abstract class AbstractSelect extends AbstractField implements } if (!selected.isEmpty()) { if (isMultiSelect()) { - setValue(selected); + setValue(selected, false, true); } else if (selected.size() == 1) { - setValue(selected.iterator().next()); + setValue(selected.iterator().next(), false, true); } else { throw new DesignException( "Multiple values selected for a single select component"); diff --git a/server/src/com/vaadin/ui/CheckBox.java b/server/src/com/vaadin/ui/CheckBox.java index e98a2b61b9..8b31edcbb4 100644 --- a/server/src/com/vaadin/ui/CheckBox.java +++ b/server/src/com/vaadin/ui/CheckBox.java @@ -221,8 +221,9 @@ public class CheckBox extends AbstractField { public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); if (design.hasAttr("checked")) { - this.setValue(DesignAttributeHandler.readAttribute("checked", - design.attributes(), Boolean.class)); + this.setValue( + DesignAttributeHandler.readAttribute("checked", + design.attributes(), Boolean.class), false, true); } } diff --git a/server/src/com/vaadin/ui/DateField.java b/server/src/com/vaadin/ui/DateField.java index 422b1ffdd8..052539cd28 100644 --- a/server/src/com/vaadin/ui/DateField.java +++ b/server/src/com/vaadin/ui/DateField.java @@ -1078,7 +1078,7 @@ public class DateField extends AbstractField implements Logger.getLogger(DateField.class.getName()).info( "cannot parse " + design.attr("value") + " as date"); } - this.setValue(date); + this.setValue(date, false, true); } } diff --git a/server/src/com/vaadin/ui/PasswordField.java b/server/src/com/vaadin/ui/PasswordField.java index ff3b1fea1f..385649c318 100644 --- a/server/src/com/vaadin/ui/PasswordField.java +++ b/server/src/com/vaadin/ui/PasswordField.java @@ -94,7 +94,7 @@ public class PasswordField extends AbstractTextField { Attributes attr = design.attributes(); if (attr.hasKey("value")) { setValue(DesignAttributeHandler.readAttribute("value", attr, - String.class)); + String.class), false, true); } } diff --git a/server/src/com/vaadin/ui/ProgressBar.java b/server/src/com/vaadin/ui/ProgressBar.java index 89baac1e64..d38067d51e 100644 --- a/server/src/com/vaadin/ui/ProgressBar.java +++ b/server/src/com/vaadin/ui/ProgressBar.java @@ -159,8 +159,9 @@ public class ProgressBar extends AbstractField implements public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); if (design.hasAttr("value") && !design.attr("value").isEmpty()) { - setValue(DesignAttributeHandler.readAttribute("value", - design.attributes(), Float.class)); + setValue( + DesignAttributeHandler.readAttribute("value", + design.attributes(), Float.class), false, true); } } diff --git a/server/src/com/vaadin/ui/RichTextArea.java b/server/src/com/vaadin/ui/RichTextArea.java index 7c23cce5cb..eff669c4c6 100644 --- a/server/src/com/vaadin/ui/RichTextArea.java +++ b/server/src/com/vaadin/ui/RichTextArea.java @@ -300,7 +300,7 @@ public class RichTextArea extends AbstractField implements @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - setValue(design.html()); + setValue(design.html(), false, true); } @Override diff --git a/server/src/com/vaadin/ui/Slider.java b/server/src/com/vaadin/ui/Slider.java index 15c94b6d3c..6b0880efd5 100644 --- a/server/src/com/vaadin/ui/Slider.java +++ b/server/src/com/vaadin/ui/Slider.java @@ -370,9 +370,10 @@ public class Slider extends AbstractField { if (attr.hasKey("vertical")) { setOrientation(SliderOrientation.VERTICAL); } - if (!attr.get("value").isEmpty()) { - setValue(DesignAttributeHandler.readAttribute("value", attr, - Double.class)); + if (attr.hasKey("value")) { + Double newFieldValue = DesignAttributeHandler.readAttribute( + "value", attr, Double.class); + setValue(newFieldValue, false, true); } } diff --git a/server/src/com/vaadin/ui/TextArea.java b/server/src/com/vaadin/ui/TextArea.java index 75ecc19d40..c2af125338 100644 --- a/server/src/com/vaadin/ui/TextArea.java +++ b/server/src/com/vaadin/ui/TextArea.java @@ -146,7 +146,8 @@ public class TextArea extends AbstractTextField { @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - setValue(DesignFormatter.unencodeFromTextNode(design.html())); + setValue(DesignFormatter.unencodeFromTextNode(design.html()), false, + true); } /* diff --git a/server/src/com/vaadin/ui/TextField.java b/server/src/com/vaadin/ui/TextField.java index 2a61e93211..8772f95f3d 100644 --- a/server/src/com/vaadin/ui/TextField.java +++ b/server/src/com/vaadin/ui/TextField.java @@ -115,8 +115,9 @@ public class TextField extends AbstractTextField { super.readDesign(design, designContext); Attributes attr = design.attributes(); if (attr.hasKey("value")) { - setValue(DesignAttributeHandler.readAttribute("value", attr, - String.class)); + String newFieldValue = DesignAttributeHandler.readAttribute( + "value", attr, String.class); + setValue(newFieldValue, false, true); } } diff --git a/server/tests/src/com/vaadin/tests/server/component/checkbox/CheckboxDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/checkbox/CheckboxDeclarativeTest.java index 6162e41494..e718c9b2dd 100644 --- a/server/tests/src/com/vaadin/tests/server/component/checkbox/CheckboxDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/checkbox/CheckboxDeclarativeTest.java @@ -44,4 +44,14 @@ public class CheckboxDeclarativeTest extends DeclarativeTestBase { testRead(design, checkBox); testWrite(design, checkBox); } + + @Test + public void testReadOnlyValue() { + String design = ""; + CheckBox checkBox = new CheckBox(); + checkBox.setValue(true); + checkBox.setReadOnly(true); + testRead(design, checkBox); + testWrite(design, checkBox); + } } diff --git a/server/tests/src/com/vaadin/tests/server/component/combobox/ComboBoxDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/combobox/ComboBoxDeclarativeTest.java index e9d66b478b..fe501f8d3e 100644 --- a/server/tests/src/com/vaadin/tests/server/component/combobox/ComboBoxDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/combobox/ComboBoxDeclarativeTest.java @@ -57,6 +57,22 @@ public class ComboBoxDeclarativeTest extends DeclarativeTestBase { testWrite(getBasicDesign(), getBasicExpected()); } + @Test + public void testReadOnlyValue() { + String design = ""; + + ComboBox comboBox = new ComboBox(); + comboBox.addItems("foo", "bar"); + comboBox.setValue("foo"); + comboBox.setReadOnly(true); + + testRead(design, comboBox); + + // Selects items are not written out by default + String design2 = ""; + testWrite(design2, comboBox); + } + private String getBasicDesign() { return ""; } diff --git a/server/tests/src/com/vaadin/tests/server/component/datefield/DateFieldDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/datefield/DateFieldDeclarativeTest.java index 5058cf5a5f..4f93930227 100644 --- a/server/tests/src/com/vaadin/tests/server/component/datefield/DateFieldDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/datefield/DateFieldDeclarativeTest.java @@ -87,4 +87,16 @@ public class DateFieldDeclarativeTest extends DeclarativeTestBase { "2020-01-01 00:00:00+0200"), getYearResolutionExpected()); } + + @Test + public void testReadOnlyValue() { + String design = ""; + DateField df = new DateField(); + df.setResolution(Resolution.YEAR); + df.setValue(new Date(2020 - 1900, 1 - 1, 1)); + df.setReadOnly(true); + + testRead(design, df); + testWrite(design, df); + } } diff --git a/server/tests/src/com/vaadin/tests/server/component/nativeselect/NativeSelectDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/nativeselect/NativeSelectDeclarativeTest.java index 8f1b995cc6..7d9e938190 100644 --- a/server/tests/src/com/vaadin/tests/server/component/nativeselect/NativeSelectDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/nativeselect/NativeSelectDeclarativeTest.java @@ -50,4 +50,20 @@ public class NativeSelectDeclarativeTest extends testWrite(stripOptionTags(getBasicDesign()), getBasicExpected()); } + @Test + public void testReadOnlyValue() { + String design = ""; + + NativeSelect ns = new NativeSelect(); + ns.addItems("foo", "bar"); + ns.setValue("foo"); + ns.setReadOnly(true); + + testRead(design, ns); + + // Selects items are not written out by default + String design2 = ""; + testWrite(design2, ns); + } + } \ No newline at end of file diff --git a/server/tests/src/com/vaadin/tests/server/component/passwordfield/PasswordFieldDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/passwordfield/PasswordFieldDeclarativeTest.java new file mode 100644 index 0000000000..615664745d --- /dev/null +++ b/server/tests/src/com/vaadin/tests/server/component/passwordfield/PasswordFieldDeclarativeTest.java @@ -0,0 +1,40 @@ +/* + * Copyright 2000-2014 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.tests.server.component.passwordfield; + +import org.junit.Test; + +import com.vaadin.tests.design.DeclarativeTestBase; +import com.vaadin.ui.PasswordField; + +/** + * + * @since + * @author Vaadin Ltd + */ +public class PasswordFieldDeclarativeTest extends + DeclarativeTestBase { + + @Test + public void testReadOnlyValue() { + String design = ""; + PasswordField tf = new PasswordField(); + tf.setValue("test value"); + tf.setReadOnly(true); + testRead(design, tf); + testWrite(design, tf); + } +} diff --git a/server/tests/src/com/vaadin/tests/server/component/progressbar/ProgressBarDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/progressbar/ProgressBarDeclarativeTest.java index c98883a4a7..e72b918497 100644 --- a/server/tests/src/com/vaadin/tests/server/component/progressbar/ProgressBarDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/progressbar/ProgressBarDeclarativeTest.java @@ -60,4 +60,16 @@ public class ProgressBarDeclarativeTest extends testWrite("", new ProgressBar()); } -} \ No newline at end of file + @Test + public void testReadOnlyValue() { + String design = ""; + ProgressBar progressBar = new ProgressBar(); + progressBar.setIndeterminate(true); + progressBar.setValue(0.5f); + progressBar.setReadOnly(true); + + testRead(design, progressBar); + testWrite(design, progressBar); + } + +} diff --git a/server/tests/src/com/vaadin/tests/server/component/richtextarea/RichTextAreaDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/richtextarea/RichTextAreaDeclarativeTest.java index 9d61656801..590ea85920 100644 --- a/server/tests/src/com/vaadin/tests/server/component/richtextarea/RichTextAreaDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/richtextarea/RichTextAreaDeclarativeTest.java @@ -56,4 +56,15 @@ public class RichTextAreaDeclarativeTest extends public void testWriteEmpty() { testWrite("", new RichTextArea()); } + + @Test + public void testReadOnlyValue() { + String design = "Hello World!"; + RichTextArea ta = new RichTextArea(); + ta.setValue("Hello World!"); + ta.setReadOnly(true); + + testRead(design, ta); + testWrite(design, ta); + } } diff --git a/server/tests/src/com/vaadin/tests/server/component/slider/SliderDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/slider/SliderDeclarativeTest.java index 9ef28afb82..eb01079cd8 100644 --- a/server/tests/src/com/vaadin/tests/server/component/slider/SliderDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/slider/SliderDeclarativeTest.java @@ -63,4 +63,19 @@ public class SliderDeclarativeTest extends DeclarativeTestBase { testRead(design, expected); testWrite(design, expected); } + + @Test + public void testReadOnlyValue() { + String design = ""; + + Slider expected = new Slider(); + expected.setMin(10.0); + expected.setMax(20.0); + expected.setResolution(1); + expected.setValue(12.3); + expected.setReadOnly(true); + + testRead(design, expected); + testWrite(design, expected); + } } diff --git a/server/tests/src/com/vaadin/tests/server/component/textarea/TextAreaDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/textarea/TextAreaDeclarativeTest.java index bed8851204..1b2de97e85 100644 --- a/server/tests/src/com/vaadin/tests/server/component/textarea/TextAreaDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/textarea/TextAreaDeclarativeTest.java @@ -58,6 +58,17 @@ public class TextAreaDeclarativeTest extends DeclarativeTestBase