From d8d4cbafb4d33994e9baedebceb8cbb7bebab791 Mon Sep 17 00:00:00 2001
From: Artur Signell <artur.signell@itmill.com>
Date: Thu, 22 Apr 2010 11:11:18 +0000
Subject: [PATCH] Test case and fix for #4528 - PaintException if the last
 menuItem of a sub menu is invisible

svn changeset:12765/svn branch:6.3
---
 src/com/vaadin/ui/MenuBar.java                | 77 +++++++++----------
 .../menubar/HiddenAndDisabledMenus.java       | 34 ++++++++
 2 files changed, 69 insertions(+), 42 deletions(-)
 create mode 100644 tests/src/com/vaadin/tests/components/menubar/HiddenAndDisabledMenus.java

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