Test case and fix for #4528 - PaintException if the last menuItem of a sub menu is...

svn changeset:12765/svn branch:6.3

diff --git a/src/com/vaadin/ui/MenuBar.java b/src/com/vaadin/ui/MenuBar.java
index 03057c62072a02937fb8c41dbd2e57ddef657360..036aa1b7180a954a35aff8338b37053e4ec239ee 100644 (file)
--- a/src/com/vaadin/ui/MenuBar.java
+++ b/src/com/vaadin/ui/MenuBar.java
@@ -76,63 +76,56 @@ public class MenuBar extends AbstractComponent {
         target.endTag("options");
         target.startTag("items");
 
-        Iterator<MenuItem> itr = menuItems.iterator();
-
         // This generates the tree from the contents of the menu
-        while (itr.hasNext()) {
-
-            MenuItem item = itr.next();
-            if (!item.isVisible()) {
-                continue;
-            }
+        for (MenuItem item : menuItems) {
+            paintItem(target, item);
+        }
 
-            target.startTag("item");
-            target.addAttribute("id", item.getId());
+        target.endTag("items");
+    }
 
-            if (item.getStyleName() != null) {
-                target.addAttribute("style", item.getStyleName());
-            }
+    private void paintItem(PaintTarget target, MenuItem item)
+            throws PaintException {
+        if (!item.isVisible()) {
+            return;
+        }
 
-            if (item.isSeparator()) {
-                target.addAttribute("separator", true);
-                target.endTag("item");
-            } else {
-                target.addAttribute("text", item.getText());
+        target.startTag("item");
 
-                Command command = item.getCommand();
-                if (command != null) {
-                    target.addAttribute("command", true);
-                }
+        target.addAttribute("id", item.getId());
 
-                Resource icon = item.getIcon();
-                if (icon != null) {
-                    target.addAttribute("icon", icon);
-                }
+        if (item.getStyleName() != null) {
+            target.addAttribute("style", item.getStyleName());
+        }
 
-                if (!item.isEnabled()) {
-                    target.addAttribute("disabled", true);
-                }
+        if (item.isSeparator()) {
+            target.addAttribute("separator", true);
+        } else {
+            target.addAttribute("text", item.getText());
 
-                if (item.hasChildren()) {
-                    iteratorStack.push(itr); // For later use
+            Command command = item.getCommand();
+            if (command != null) {
+                target.addAttribute("command", true);
+            }
 
-                    // Go through the children
-                    itr = item.getChildren().iterator();
-                } else {
-                    target.endTag("item"); // Item had no children, end
-                    // description
-                }
+            Resource icon = item.getIcon();
+            if (icon != null) {
+                target.addAttribute("icon", icon);
+            }
 
+            if (!item.isEnabled()) {
+                target.addAttribute("disabled", true);
             }
 
-            // The end submenu. More than one submenu may end at once.
-            while (!itr.hasNext() && !iteratorStack.empty()) {
-                itr = iteratorStack.pop();
-                target.endTag("item");
+            if (item.hasChildren()) {
+                for (MenuItem child : item.getChildren()) {
+                    paintItem(target, child);
+                }
             }
+
         }
 
-        target.endTag("items");
+        target.endTag("item");
     }
 
     /** Deserialize changes received from client. */

diff --git a/tests/src/com/vaadin/tests/components/menubar/HiddenAndDisabledMenus.java b/tests/src/com/vaadin/tests/components/menubar/HiddenAndDisabledMenus.java
new file mode 100644 (file)
index 0000000..ef5d5ad
--- /dev/null
+++ b/tests/src/com/vaadin/tests/components/menubar/HiddenAndDisabledMenus.java
@@ -0,0 +1,34 @@
+package com.vaadin.tests.components.menubar;\r
+\r
+import com.vaadin.tests.components.TestBase;\r
+import com.vaadin.ui.MenuBar;\r
+import com.vaadin.ui.MenuBar.MenuItem;\r
+\r
+public class HiddenAndDisabledMenus extends TestBase {\r
+\r
+    @Override\r
+    protected void setup() {\r
+        MenuBar mb = new MenuBar();\r
+        mb.addItem("Item 1", null);\r
+        mb.addItem("Item 2 - hidden", null).setVisible(false);\r
+        MenuItem submenu = mb.addItem("Item 3 - sub menu", null);\r
+        mb.addItem("Item 4 - hidden", null).setVisible(false);\r
+        submenu.addItem("Sub item 1 - disabled", null).setEnabled(false);\r
+        submenu.addItem("Sub item 2 - enabled", null);\r
+        submenu.addItem("Sub item 3 - visible", null);\r
+        submenu.addItem("Sub item 4 - hidden", null).setVisible(false);\r
+\r
+        addComponent(mb);\r
+    }\r
+\r
+    @Override\r
+    protected String getDescription() {\r
+        return "The menu contains 4 items, 2 of which are hidden. The sub menu contains 4 items, the last one is hidden";\r
+    }\r
+\r
+    @Override\r
+    protected Integer getTicketNumber() {\r
+        return 4528;\r
+    }\r
+\r
+}\r