Fix MenuBar MenuItem caption reading from design (#17361)

Change-Id: I438039ab8ad83bf0b2153939502903571f231bbb

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

@@ -1008,8 +1008,9 @@ public class MenuBar extends AbstractComponent implements LegacyComponent,
             if (node instanceof Element
                     && ((Element) node).tagName().equals("menu")) {
                 subMenus.add((Element) node);
+            } else {
+                caption += node.toString();
             }
-            caption += node.toString();
         }
         MenuItem menu = new MenuItem(caption.trim(), icon, null);
 

server/tests/src/com/vaadin/tests/components/menubar/MenuBarDeclarativeTest.java

@@ -16,7 +16,9 @@
 package com.vaadin.tests.components.menubar;
 
 import java.io.IOException;
+import java.util.List;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 import com.vaadin.server.ExternalResource;
@@ -128,7 +130,7 @@ public class MenuBarDeclarativeTest extends DeclarativeTestBase<MenuBar> {
         MenuBar menuBar = new MenuBar();
         menuBar.setHtmlContentAllowed(true);
         MenuItem fileMenu = menuBar.addItem("<b>File</b>", null);
-        fileMenu.addItem("<font style='color: red'>Save</font>", null);
+        fileMenu.addItem("<font style=\"color: red\">Save</font>", null);
         fileMenu.addItem("Open", new ThemeResource(
                 "../runo/icons/16/folder.png"), null);
         fileMenu.addSeparator();
@@ -136,4 +138,62 @@ public class MenuBarDeclarativeTest extends DeclarativeTestBase<MenuBar> {
         testRead(design, menuBar);
         testWrite(design, menuBar);
     }
+
+    @Override
+    public MenuBar testRead(String design, MenuBar expected) {
+        MenuBar result = super.testRead(design, expected);
+
+        List<MenuItem> expectedMenuItems = expected.getItems();
+        List<MenuItem> actualMenuItems = result.getItems();
+        Assert.assertEquals("Different amount of menu items",
+                expectedMenuItems.size(), actualMenuItems.size());
+
+        for (int i = 0; i < expectedMenuItems.size(); ++i) {
+            compareMenus(expectedMenuItems.get(i), actualMenuItems.get(i));
+        }
+
+        return result;
+    }
+
+    private void compareMenus(MenuItem expected, MenuItem actual) {
+        String baseError = "Error Comparing MenuItem " + expected.getText()
+                + ": ";
+        Assert.assertEquals(baseError + "Visibile", expected.isVisible(),
+                actual.isVisible());
+        Assert.assertEquals(baseError + "Checkable", expected.isCheckable(),
+                actual.isCheckable());
+        Assert.assertEquals(baseError + "Checked", expected.isChecked(),
+                actual.isChecked());
+        Assert.assertEquals(baseError + "Separator", expected.isSeparator(),
+                actual.isSeparator());
+        Assert.assertEquals(baseError + "Enabled", expected.isEnabled(),
+                actual.isEnabled());
+
+        Assert.assertEquals(baseError + "Text", expected.getText(),
+                actual.getText());
+        Assert.assertEquals(baseError + "Description",
+                expected.getDescription(), actual.getDescription());
+        Assert.assertEquals(baseError + "Style Name", expected.getStyleName(),
+                actual.getStyleName());
+
+        if (expected.getIcon() != null) {
+            Assert.assertNotNull(baseError + "Icon was null", actual.getIcon());
+        } else {
+            if (actual.getIcon() != null) {
+                Assert.fail(baseError + "Icon should've been null");
+            }
+        }
+
+        Assert.assertEquals(baseError + "Has Children", expected.hasChildren(),
+                actual.hasChildren());
+        if (expected.hasChildren()) {
+            List<MenuItem> children = expected.getChildren();
+            List<MenuItem> actualChildren = actual.getChildren();
+            Assert.assertEquals(baseError + "Child count", children.size(),
+                    actualChildren.size());
+            for (int i = 0; i < children.size(); ++i) {
+                compareMenus(children.get(i), actualChildren.get(i));
+            }
+        }
+    }
 }