Fix declarative support for Window (#17314)

Change-Id: If89a46a4c08ec1491eb00a2f2b8580fb3ef785fc

server/src/com/vaadin/ui/AbstractSingleComponentContainer.java

@@ -19,6 +19,7 @@ import java.util.Collections;
 import java.util.Iterator;
 
 import org.jsoup.nodes.Element;
+import org.jsoup.select.Elements;
 
 import com.vaadin.server.ComponentSizeValidator;
 import com.vaadin.server.VaadinService;
@@ -288,17 +289,33 @@ public abstract class AbstractSingleComponentContainer extends
     public void readDesign(Element design, DesignContext designContext) {
         // process default attributes
         super.readDesign(design, designContext);
-        // handle child element, checking that the design specifies at most one
-        // child
-        int childCount = design.children().size();
-        if (childCount > 1) {
+        readDesignChildren(design.children(), designContext);
+    }
+
+    /**
+     * Reads the content component from the list of child elements of a design.
+     * The list must be empty or contain a single element; if the design
+     * contains multiple child elements, a DesignException is thrown. This
+     * method should be overridden by subclasses whose design may contain
+     * non-content child elements.
+     *
+     * @param children
+     *            the child elements of the design that is being read
+     * @param context
+     *            the DesignContext instance used to parse the design
+     * 
+     * @throws DesignException
+     *             if there are multiple child elements
+     * @throws DesignException
+     *             if a child element could not be parsed as a Component
+     */
+    protected void readDesignChildren(Elements children, DesignContext context) {
+        if (children.size() > 1) {
             throw new DesignException("The container of type "
                     + getClass().toString()
                     + " can have only one child component.");
-        } else if (childCount == 1) {
-            Element childElement = design.children().get(0);
-            Component newChild = designContext.readDesign(childElement);
-            setContent(newChild);
+        } else if (children.size() == 1) {
+            setContent(context.readDesign(children.first()));
         }
     }
 

server/src/com/vaadin/ui/Window.java

@@ -18,11 +18,18 @@ package com.vaadin.ui;
 
 import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 
+import org.jsoup.nodes.Element;
+import org.jsoup.select.Elements;
+
 import com.vaadin.event.FieldEvents.BlurEvent;
 import com.vaadin.event.FieldEvents.BlurListener;
 import com.vaadin.event.FieldEvents.BlurNotifier;
@@ -42,6 +49,9 @@ import com.vaadin.shared.ui.window.WindowMode;
 import com.vaadin.shared.ui.window.WindowRole;
 import com.vaadin.shared.ui.window.WindowServerRpc;
 import com.vaadin.shared.ui.window.WindowState;
+import com.vaadin.ui.declarative.DesignAttributeHandler;
+import com.vaadin.ui.declarative.DesignContext;
+import com.vaadin.ui.declarative.DesignException;
 import com.vaadin.util.ReflectTools;
 
 /**
@@ -1283,4 +1293,116 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier,
     public String getTabStopBottomAssistiveText() {
         return getState(false).assistiveTabStopBottomText;
     }
+
+    @Override
+    public void readDesign(Element design, DesignContext context) {
+        super.readDesign(design, context);
+
+        if (design.hasAttr("center")) {
+            center();
+        }
+        if (design.hasAttr("position")) {
+            String[] position = design.attr("position").split(",");
+            setPositionX(Integer.parseInt(position[0]));
+            setPositionY(Integer.parseInt(position[1]));
+        }
+        if (design.hasAttr("close-shortcut")) {
+            ShortcutAction shortcut = DesignAttributeHandler
+                    .readAttribute("close-shortcut", design.attributes(),
+                            ShortcutAction.class);
+            setCloseShortcut(shortcut.getKeyCode(), shortcut.getModifiers());
+        }
+    }
+
+    /**
+     * Reads the content and possible assistive descriptions from the list of
+     * child elements of a design. If an element has an
+     * {@code :assistive-description} attribute, adds the parsed component to
+     * the list of components used as the assistive description of this Window.
+     * Otherwise, sets the component as the content of this Window. If there are
+     * multiple non-description elements, throws a DesignException.
+     * 
+     * @param children
+     *            child elements in a design
+     * @param context
+     *            the DesignContext instance used to parse the design
+     * 
+     * @throws DesignException
+     *             if there are multiple non-description child elements
+     * @throws DesignException
+     *             if a child element could not be parsed as a Component
+     * 
+     * @see #setContent(Component)
+     * @see #setAssistiveDescription(Component...)
+     */
+    @Override
+    protected void readDesignChildren(Elements children, DesignContext context) {
+        List<Component> descriptions = new ArrayList<Component>();
+        Elements content = new Elements();
+
+        for (Element child : children) {
+            if (child.hasAttr(":assistive-description")) {
+                descriptions.add(context.readDesign(child));
+            } else {
+                content.add(child);
+            }
+        }
+        super.readDesignChildren(content, context);
+        setAssistiveDescription(descriptions.toArray(new Component[0]));
+    }
+
+    @Override
+    public void writeDesign(Element design, DesignContext context) {
+        super.writeDesign(design, context);
+
+        Window def = context.getDefaultInstance(this);
+
+        if (getState().centered) {
+            design.attr("center", "");
+        }
+
+        DesignAttributeHandler.writeAttribute("position", design.attributes(),
+                getPosition(), def.getPosition(), String.class);
+
+        CloseShortcut shortcut = getCloseShortcut();
+        if (shortcut != null) {
+            // TODO What if several close shortcuts??
+
+            CloseShortcut defShortcut = def.getCloseShortcut();
+            if (defShortcut == null
+                    || shortcut.getKeyCode() != defShortcut.getKeyCode()
+                    || !Arrays.equals(shortcut.getModifiers(),
+                            defShortcut.getModifiers())) {
+                DesignAttributeHandler.writeAttribute("close-shortcut",
+                        design.attributes(), shortcut, null,
+                        CloseShortcut.class);
+            }
+        }
+
+        for (Component c : getAssistiveDescription()) {
+            Element child = context.createElement(c).attr(
+                    ":assistive-description", "");
+            design.appendChild(child);
+        }
+    }
+
+    private String getPosition() {
+        return getPositionX() + "," + getPositionY();
+    }
+
+    private CloseShortcut getCloseShortcut() {
+        Iterator<CloseShortcut> i = getCloseShortcuts().iterator();
+        return i.hasNext() ? i.next() : null;
+    }
+
+    @Override
+    protected Collection<String> getCustomAttributes() {
+        Collection<String> result = super.getCustomAttributes();
+        result.add("center");
+        result.add("position");
+        result.add("position-y");
+        result.add("position-x");
+        result.add("close-shortcut");
+        return result;
+    }
 }

server/src/com/vaadin/ui/declarative/converters/DesignShortcutActionConverter.java

@@ -126,32 +126,37 @@ public class DesignShortcutActionConverter implements
         if (value.length() == 0) {
             return null;
         }
-        String[] data = value.split(" ", 2);
 
+        String[] data = value.split(" ", 2);
         String[] parts = data[0].split("-");
-        // handle keycode
-        String keyCodePart = parts[parts.length - 1];
-        int keyCode = getKeycodeForString(keyCodePart);
-        if (keyCode < 0) {
-            throw new IllegalArgumentException("Invalid shortcut definition "
-                    + value);
-        }
-        // handle modifiers
-        int[] modifiers = null;
-        if (parts.length > 1) {
-            modifiers = new int[parts.length - 1];
-        }
-        for (int i = 0; i < parts.length - 1; i++) {
-            int modifier = getKeycodeForString(parts[i]);
-            if (modifier > 0) {
-                modifiers[i] = modifier;
-            } else {
-                throw new IllegalArgumentException(
-                        "Invalid shortcut definition " + value);
+
+        try {
+            // handle keycode
+            String keyCodePart = parts[parts.length - 1];
+            int keyCode = getKeycodeForString(keyCodePart);
+            if (keyCode < 0) {
+                throw new IllegalArgumentException("Invalid key '"
+                        + keyCodePart + "'");
+            }
+            // handle modifiers
+            int[] modifiers = null;
+            if (parts.length > 1) {
+                modifiers = new int[parts.length - 1];
+            }
+            for (int i = 0; i < parts.length - 1; i++) {
+                int modifier = getKeycodeForString(parts[i]);
+                if (modifier > 0) {
+                    modifiers[i] = modifier;
+                } else {
+                    throw new IllegalArgumentException("Invalid modifier '"
+                            + parts[i] + "'");
+                }
             }
+            return new ShortcutAction(data.length == 2 ? data[1] : null,
+                    keyCode, modifiers);
+        } catch (Exception e) {
+            throw new ConversionException("Invalid shortcut '" + value + "'", e);
         }
-        return new ShortcutAction(data.length == 2 ? data[1] : null, keyCode,
-                modifiers);
     }
 
     @Override

server/tests/src/com/vaadin/tests/design/DeclarativeTestBase.java

@@ -24,6 +24,7 @@ import java.util.Map;
 
 import org.junit.Assert;
 
+import com.vaadin.shared.Connector;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.Flash;
 
@@ -59,6 +60,11 @@ public abstract class DeclarativeTestBase<T extends Component> extends
                     if (readMethod == null || writeMethod == null) {
                         continue;
                     }
+                    if (Connector.class.isAssignableFrom(c)
+                            && readMethod.getName().equals("getParent")) {
+                        // Hack to break cycles in the connector hierarchy
+                        continue;
+                    }
                     try {
                         c.getDeclaredMethod(readMethod.getName());
                     } catch (Exception e) {
@@ -99,7 +105,6 @@ public abstract class DeclarativeTestBase<T extends Component> extends
                 }
             }
         });
-
     }
 
     @Override
@@ -118,5 +123,4 @@ public abstract class DeclarativeTestBase<T extends Component> extends
         }
         return comp;
     }
-
 }

server/tests/src/com/vaadin/tests/design/DeclarativeTestBaseBase.java

@@ -84,12 +84,21 @@ public abstract class DeclarativeTestBaseBase<T extends Component> {
             return;
         }
 
-        if (o1 instanceof Collection && o2 instanceof Collection) {
-
-        } else {
+        if (!(o1 instanceof Collection && o2 instanceof Collection)) {
             Assert.assertEquals(o1.getClass(), o2.getClass());
         }
 
+        if (o1 instanceof Object[]) {
+            Object[] a1 = ((Object[]) o1);
+            Object[] a2 = ((Object[]) o2);
+            Assert.assertEquals(message + ": array length", a1.length,
+                    a2.length);
+            for (int i = 0; i < a1.length; i++) {
+                assertEquals(message, a1[i], a2[i]);
+            }
+            return;
+        }
+
         List<EqualsAsserter<Object>> comparators = getComparators(o1);
         if (!comparators.isEmpty()) {
             for (EqualsAsserter<Object> ec : comparators) {
@@ -126,7 +135,9 @@ public abstract class DeclarativeTestBaseBase<T extends Component> {
     protected abstract <TT> EqualsAsserter<TT> getComparator(Class<TT> c);
 
     private boolean isVaadin(Class<?> c) {
-        return c.getPackage().getName().startsWith("com.vaadin");
+        return c.getPackage() != null
+                && c.getPackage().getName().startsWith("com.vaadin");
+
     }
 
     public void testRead(String design, T expected) {
@@ -149,6 +160,10 @@ public abstract class DeclarativeTestBaseBase<T extends Component> {
         Assert.assertEquals(comparable, produced);
     }
 
+    protected Element createElement(Component c) {
+        return new DesignContext().createElement(c);
+    }
+
     private String elementToHtml(Element producedElem) {
         StringBuilder stringBuilder = new StringBuilder();
         elementToHtml(producedElem, stringBuilder);

server/tests/src/com/vaadin/tests/design/DesignFormatterTest.java

@@ -24,10 +24,14 @@ import java.util.Date;
 import java.util.HashSet;
 import java.util.TimeZone;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.vaadin.data.util.converter.Converter.ConversionException;
 import com.vaadin.event.ShortcutAction;
+import com.vaadin.event.ShortcutAction.KeyCode;
+import com.vaadin.event.ShortcutAction.ModifierKey;
 import com.vaadin.server.ExternalResource;
 import com.vaadin.server.FileResource;
 import com.vaadin.server.Resource;
@@ -203,10 +207,8 @@ public class DesignFormatterTest {
 
     @Test
     public void testShortcutActionNoCaption() {
-        ShortcutAction action = new ShortcutAction(null,
-                ShortcutAction.KeyCode.D, new int[] {
-                        ShortcutAction.ModifierKey.ALT,
-                        ShortcutAction.ModifierKey.CTRL });
+        ShortcutAction action = new ShortcutAction(null, KeyCode.D, new int[] {
+                ModifierKey.ALT, ModifierKey.CTRL });
         String formatted = formatter.format(action);
         assertEquals("alt-ctrl-d", formatted);
 
@@ -215,6 +217,23 @@ public class DesignFormatterTest {
         assertTrue(equals(action, result));
     }
 
+    @Test
+    public void testInvalidShortcutAction() {
+        assertInvalidShortcut("-");
+        assertInvalidShortcut("foo");
+        assertInvalidShortcut("atl-ctrl");
+        assertInvalidShortcut("-a");
+    }
+
+    protected void assertInvalidShortcut(String shortcut) {
+        try {
+            formatter.parse(shortcut, ShortcutAction.class);
+            Assert.fail("Invalid shortcut '" + shortcut + "' should throw");
+        } catch (ConversionException e) {
+            // expected
+        }
+    }
+
     @Test
     public void testTimeZone() {
         TimeZone zone = TimeZone.getTimeZone("GMT+2");

server/tests/src/com/vaadin/tests/server/component/window/WindowDeclarativeTest.java

@@ -0,0 +1,153 @@
+/*
+ * 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.window;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.event.ShortcutAction.KeyCode;
+import com.vaadin.event.ShortcutAction.ModifierKey;
+import com.vaadin.shared.ui.window.WindowMode;
+import com.vaadin.shared.ui.window.WindowRole;
+import com.vaadin.tests.design.DeclarativeTestBase;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Window;
+import com.vaadin.ui.declarative.DesignException;
+
+/**
+ * Tests declarative support for implementations of {@link Window}.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class WindowDeclarativeTest extends DeclarativeTestBase<Window> {
+
+    @Test
+    public void testDefault() {
+        String design = "<v-window>";
+
+        Window expected = new Window();
+
+        testRead(design, expected);
+        testWrite(design, expected);
+    }
+
+    @Test
+    public void testFeatures() {
+
+        String design = "<v-window position='100,100' window-mode='maximized' "
+                + "center modal=true resizable=false resize-lazy=true closable=false draggable=false "
+                + "close-shortcut='ctrl-alt-escape' "
+                + "assistive-prefix='Hello' assistive-postfix='World' assistive-role='alertdialog' "
+                + "tab-stop-enabled=true "
+                + "tab-stop-top-assistive-text='Do not move above the window' "
+                + "tab-stop-bottom-assistive-text='End of window'>"
+                + "</v-window>";
+
+        Window expected = new Window();
+
+        expected.setPositionX(100);
+        expected.setPositionY(100);
+        expected.setWindowMode(WindowMode.MAXIMIZED);
+
+        expected.center();
+        expected.setModal(!expected.isModal());
+        expected.setResizable(!expected.isResizable());
+        expected.setResizeLazy(!expected.isResizeLazy());
+        expected.setClosable(!expected.isClosable());
+        expected.setDraggable(!expected.isDraggable());
+
+        expected.setCloseShortcut(KeyCode.ESCAPE, ModifierKey.CTRL,
+                ModifierKey.ALT);
+
+        expected.setAssistivePrefix("Hello");
+        expected.setAssistivePostfix("World");
+        expected.setAssistiveRole(WindowRole.ALERTDIALOG);
+        expected.setTabStopEnabled(!expected.isTabStopEnabled());
+        expected.setTabStopTopAssistiveText("Do not move above the window");
+        expected.setTabStopBottomAssistiveText("End of window");
+
+        testRead(design, expected);
+        testWrite(design, expected);
+    }
+
+    @Test
+    public void testInvalidPosition() {
+        assertInvalidPosition("");
+        assertInvalidPosition("1");
+        assertInvalidPosition("100,100.1");
+        assertInvalidPosition("x");
+        assertInvalidPosition("2,foo");
+        // Should be invalid, not checked currently
+        // assertInvalidPosition("1,2,3");
+    }
+
+    protected void assertInvalidPosition(String position) {
+        try {
+            read("<v-window position='" + position + "'>");
+            Assert.fail("Invalid position '" + position + "' should throw");
+        } catch (Exception e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testChildContent() {
+
+        String design = "<v-window>" + createElement(new Button("OK"))
+                + "</v-window>";
+
+        Window expected = new Window();
+        expected.setContent(new Button("OK"));
+
+        testRead(design, expected);
+        testWrite(design, expected);
+    }
+
+    @Test(expected = DesignException.class)
+    public void testMultipleContentChildren() {
+
+        String design = "<v-window>" + createElement(new Label("Hello"))
+                + createElement(new Button("OK")) + "</v-window>";
+
+        read(design);
+    }
+
+    @Test
+    public void testAssistiveDescription() {
+
+        Label assistive1 = new Label("Assistive text");
+        Label assistive2 = new Label("More assistive text");
+
+        String design = "<v-window>"
+                + createElement(assistive1).attr(":assistive-description", "")
+                + createElement(new Button("OK"))
+                + createElement(assistive2).attr(":assistive-description", "");
+
+        Window expected = new Window();
+        expected.setContent(new Button("OK"));
+        expected.setAssistiveDescription(assistive1, assistive2);
+
+        testRead(design, expected);
+
+        String written = "<v-window>" + createElement(new Button("OK"))
+                + createElement(assistive1).attr(":assistive-description", "")
+                + createElement(assistive2).attr(":assistive-description", "");
+
+        testWrite(written, expected);
+    }
+}