Do not modify XmlTag for palette previews

Fixes: 236687661
Test: Added unit test
Change-Id: I68bbe40e6a15421cebed6bd20fecd97192a64d50

13 files changed, 88 insertions, 37 deletions

diff --git a/adt-ui/src/test/java/com/android/tools/adtui/swing/popup/FakeJBPopup.kt b/adt-ui/src/test/java/com/android/tools/adtui/swing/popup/FakeJBPopup.kt
index 27c6f3d3662..a77133c135d 100644
--- a/adt-ui/src/test/java/com/android/tools/adtui/swing/popup/FakeJBPopup.kt
+++ b/adt-ui/src/test/java/com/android/tools/adtui/swing/popup/FakeJBPopup.kt
@@ -107,6 +107,10 @@ open class FakeJBPopup<T>(
     showArgs = listOf(project)
   }
 
+  override fun isFocused(): Boolean {
+    return true
+  }
+
   override fun getBestPositionFor(dataContext: DataContext): Point {
     TODO("Not yet implemented")
   }
@@ -175,10 +179,6 @@ open class FakeJBPopup<T>(
     TODO("Not yet implemented")
   }
 
-  override fun isFocused(): Boolean {
-    TODO("Not yet implemented")
-  }
-
   override fun isCancelKeyEnabled(): Boolean {
     TODO("Not yet implemented")
   }
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/ImageViewHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/ImageViewHandler.java
index 48ab884522d..5dfedc42895 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/ImageViewHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/ImageViewHandler.java
@@ -107,12 +107,6 @@ public class ImageViewHandler extends ViewHandler {
       }
       return showImageChooser(newChild);
     }
-
-    // Fallback if dismissed or during previews etc
-    if (insertType.isCreate()) {
-      setSrcAttribute(newChild, getSampleImageSrc());
-    }
-
     return true;
   }
 
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/NestedScrollViewHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/NestedScrollViewHandler.java
index 214107df8f3..0e1e9d30f11 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/NestedScrollViewHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/NestedScrollViewHandler.java
@@ -52,7 +52,7 @@ public class NestedScrollViewHandler extends ScrollViewHandler {
       return false;
     }
 
-    if (type.isCreate()) {
+    if (type == InsertType.CREATE) {
       NlWriteCommandActionUtil.run(newChild, "Setting fill_viewport", () -> {
         newChild.setAndroidAttribute(ATTR_FILL_VIEWPORT, "true");
       });
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/ScrollViewHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/ScrollViewHandler.java
index a26765ac84c..8166118f9f1 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/ScrollViewHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/ScrollViewHandler.java
@@ -64,7 +64,7 @@ public class ScrollViewHandler extends ViewGroupHandler {
   public boolean onCreate(@Nullable NlComponent parent,
                           @NotNull NlComponent node,
                           @NotNull InsertType insertType) {
-    if (insertType.isCreate()) {
+    if (insertType == InsertType.CREATE) {
       // Insert a default linear layout (which will in turn be registered as
       // a child of this node and the create child method above will set its
       // fill parent attributes, its id, etc.
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/TableLayoutHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/TableLayoutHandler.java
index b816e4ef849..fe5bd84309c 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/TableLayoutHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/TableLayoutHandler.java
@@ -60,7 +60,7 @@ public class TableLayoutHandler extends LinearLayoutHandler {
   public boolean onCreate(@Nullable NlComponent parent,
                           @NotNull NlComponent node,
                           @NotNull InsertType insertType) {
-    if (insertType.isCreate()) {
+    if (insertType == InsertType.CREATE) {
       // Start the table with 4 rows
       NlWriteCommandActionUtil.run(node, "", () -> {
         for (int i = 0; i < 4; i++) {
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/CheckBoxPreferenceHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/CheckBoxPreferenceHandler.java
index c79a1024bff..ba029ccb2f6 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/CheckBoxPreferenceHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/CheckBoxPreferenceHandler.java
@@ -76,9 +76,11 @@ public final class CheckBoxPreferenceHandler extends PreferenceHandler {
       return false;
     }
 
-    NlWriteCommandActionUtil.run(newChild, "Set Preference Key", () -> {
-      newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, CHECK_BOX_PREFERENCE, "check_box_preference_"));
-    });
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Set Preference Key", () -> {
+        newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, CHECK_BOX_PREFERENCE, "check_box_preference_"));
+      });
+    }
     return true;
   }
 }
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/EditTextPreferenceHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/EditTextPreferenceHandler.java
index b506f16fc28..e15df11ad57 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/EditTextPreferenceHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/EditTextPreferenceHandler.java
@@ -76,9 +76,11 @@ public final class EditTextPreferenceHandler extends PreferenceHandler {
       return false;
     }
 
-    NlWriteCommandActionUtil.run(newChild, "Set EditTextPreference", () -> {
-      newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, EDIT_TEXT_PREFERENCE, "edit_text_preference_"));
-    });
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Set EditTextPreference", () -> {
+        newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, EDIT_TEXT_PREFERENCE, "edit_text_preference_"));
+      });
+    }
     return true;
   }
 }
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/PreferenceCategoryHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/PreferenceCategoryHandler.java
index db68f570068..37801e88d51 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/PreferenceCategoryHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/PreferenceCategoryHandler.java
@@ -63,10 +63,12 @@ public final class PreferenceCategoryHandler extends ViewGroupHandler {
   public boolean onCreate(@Nullable NlComponent parent,
                           @NotNull NlComponent newChild,
                           @NotNull InsertType insertType) {
-    NlWriteCommandActionUtil.run(newChild, "Clearing Width and Height", () -> {
-      newChild.removeAndroidAttribute(ATTR_LAYOUT_WIDTH);
-      newChild.removeAndroidAttribute(ATTR_LAYOUT_HEIGHT);
-    });
+    if (insertType == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Clearing Width and Height", () -> {
+        newChild.removeAndroidAttribute(ATTR_LAYOUT_WIDTH);
+        newChild.removeAndroidAttribute(ATTR_LAYOUT_HEIGHT);
+      });
+    }
 
     return true;
   }
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/RingtonePreferenceHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/RingtonePreferenceHandler.java
index beb5020bcee..78319e2a64c 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/RingtonePreferenceHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/RingtonePreferenceHandler.java
@@ -73,9 +73,11 @@ public final class RingtonePreferenceHandler extends PreferenceHandler {
       return false;
     }
 
-    NlWriteCommandActionUtil.run(newChild, "Set RingtonePreference", () -> {
-      newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, RINGTONE_PREFERENCE, "ringtone_preference_"));
-    });
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Set RingtonePreference", () -> {
+        newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, RINGTONE_PREFERENCE, "ringtone_preference_"));
+      });
+    }
     return true;
   }
 }
diff --git a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/SwitchPreferenceHandler.java b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/SwitchPreferenceHandler.java
index f6a9a5b492d..d3411ef72cf 100644
--- a/designer/src/com/android/tools/idea/uibuilder/handlers/preference/SwitchPreferenceHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/handlers/preference/SwitchPreferenceHandler.java
@@ -78,9 +78,11 @@ public final class SwitchPreferenceHandler extends PreferenceHandler {
       return false;
     }
 
-    NlWriteCommandActionUtil.run(newChild, "Set SwitchPreference", () -> {
-      newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, SWITCH_PREFERENCE, "switch_preference_"));
-    });
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Set SwitchPreference", () -> {
+        newChild.setAndroidAttribute(ATTR_KEY, generateKey(newChild, SWITCH_PREFERENCE, "switch_preference_"));
+      });
+    }
     return true;
   }
 }
diff --git a/designer/src/com/android/tools/idea/uibuilder/menu/MenuHandler.java b/designer/src/com/android/tools/idea/uibuilder/menu/MenuHandler.java
index 593c627179e..a3f97b94e45 100644
--- a/designer/src/com/android/tools/idea/uibuilder/menu/MenuHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/menu/MenuHandler.java
@@ -53,12 +53,13 @@ public class MenuHandler extends ViewGroupHandler {
   public boolean onCreate(@Nullable NlComponent parent,
                           @NotNull NlComponent newChild,
                           @NotNull InsertType type) {
-    return NlWriteCommandActionUtil.compute(newChild, "Create Menu",
-      () -> {
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Create Menu", () -> {
         newChild.removeAndroidAttribute(ATTR_LAYOUT_WIDTH);
         newChild.removeAndroidAttribute(ATTR_LAYOUT_HEIGHT);
-        return true;
-    });
+      });
+    }
+    return true;
   }
 
   @Override
diff --git a/designer/src/com/android/tools/idea/uibuilder/statelist/ItemHandler.java b/designer/src/com/android/tools/idea/uibuilder/statelist/ItemHandler.java
index 01ebe9c58e8..5e9c1d965fe 100644
--- a/designer/src/com/android/tools/idea/uibuilder/statelist/ItemHandler.java
+++ b/designer/src/com/android/tools/idea/uibuilder/statelist/ItemHandler.java
@@ -16,6 +16,7 @@
 package com.android.tools.idea.uibuilder.statelist;
 
 import com.android.tools.idea.common.api.InsertType;
+import com.android.tools.idea.common.command.NlWriteCommandActionUtil;
 import com.android.tools.idea.uibuilder.api.ViewHandler;
 import com.android.tools.idea.common.model.NlComponent;
 import com.google.common.base.Strings;
@@ -34,9 +35,12 @@ public final class ItemHandler extends ViewHandler {
   public boolean onCreate(@Nullable NlComponent parent,
                           @NotNull NlComponent newChild,
                           @NotNull InsertType type) {
-    newChild.removeAndroidAttribute(ATTR_LAYOUT_WIDTH);
-    newChild.removeAndroidAttribute(ATTR_LAYOUT_HEIGHT);
-
+    if (type == InsertType.CREATE) {
+      NlWriteCommandActionUtil.run(newChild, "Create Menu Item", () -> {
+        newChild.removeAndroidAttribute(ATTR_LAYOUT_WIDTH);
+        newChild.removeAndroidAttribute(ATTR_LAYOUT_HEIGHT);
+      });
+    }
     return true;
   }
 
diff --git a/designer/testSrc/com/android/tools/idea/uibuilder/palette/PalettePanelTest.kt b/designer/testSrc/com/android/tools/idea/uibuilder/palette/PalettePanelTest.kt
index 35c3daa9f0c..9b0cd4a0b6d 100644
--- a/designer/testSrc/com/android/tools/idea/uibuilder/palette/PalettePanelTest.kt
+++ b/designer/testSrc/com/android/tools/idea/uibuilder/palette/PalettePanelTest.kt
@@ -19,6 +19,7 @@ import com.android.SdkConstants
 import com.android.ide.common.repository.GradleCoordinate
 import com.android.testutils.MockitoKt.eq
 import com.android.testutils.MockitoKt.mock
+import com.android.tools.adtui.swing.FakeUi
 import com.android.tools.adtui.swing.popup.FakeJBPopupFactory
 import com.android.tools.adtui.swing.popup.JBPopupRule
 import com.android.tools.adtui.workbench.PropertiesComponentMock
@@ -46,8 +47,11 @@ import com.intellij.ide.util.PropertiesComponent
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.DataContext
 import com.intellij.openapi.actionSystem.PlatformDataKeys
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.application.ex.ApplicationEx
 import com.intellij.openapi.ide.CopyPasteManager
 import com.intellij.openapi.module.Module
+import com.intellij.openapi.progress.util.ProgressIndicatorUtils
 import com.intellij.openapi.project.DumbServiceImpl
 import com.intellij.openapi.ui.MessageType
 import com.intellij.openapi.ui.popup.JBPopupFactory
@@ -418,6 +422,44 @@ public class MyWebView extends android.webkit.WebView {
     assertThat(itemList.emptyText.secondaryComponent.getCharSequence(false)).isEqualTo("")
   }
 
+  @Test
+  fun testMenuCreationForLayouts() {
+    checkPopupMenuCreation(LayoutFileType)
+  }
+
+  @Test
+  fun testMenuCreationForPreferences() {
+    checkPopupMenuCreation(PreferenceScreenFileType)
+  }
+
+  @Test
+  fun testMenuCreationForMenus() {
+    checkPopupMenuCreation(MenuFileType)
+  }
+
+  private fun checkPopupMenuCreation(layoutType: DesignerEditorFileType) {
+    val ui = FakeUi(myPanel!!, createFakeWindow = true)
+    myPanel!!.setSize(800, 1000)
+    doLayout(myPanel!!)
+    createDesignSurface(layoutType)
+    val categoryList = myPanel!!.categoryList
+    for (categoryIndex in 0 until categoryList.model.size) {
+      categoryList.selectedIndex = categoryIndex
+      val itemList = myPanel!!.itemList
+      for (itemIndex in 0 until itemList.model.size) {
+        val bounds = itemList.getCellBounds(itemIndex, itemIndex)
+        val x = bounds.x + bounds.width / 2
+        val y = bounds.y + bounds.height / 2
+        val app = ApplicationManager.getApplication() as ApplicationEx
+        // During the menu popup and MenuGroup.update, we are not allowed to write to PSI.
+        // At runtime this is checked because an async DataContext is used in ActionUpdater.expandActionGroupAsync.
+        // Simulate that here by adding the no write check up front:
+        @Suppress("UnstableApiUsage")
+        ProgressIndicatorUtils.runActionAndCancelBeforeWrite(app, { error("No writes allowed") }, { ui.mouse.rightClick(x, y) })
+      }
+    }
+  }
+
   private fun registerMockStatusBar(): StatusBarEx {
     val windowManager: WindowManager = mock()
     val frame: IdeFrame = mock()
@@ -466,7 +508,7 @@ public class MyWebView extends android.webkit.WebView {
     val (resourceFolder, name) = when(layoutType) {
       LayoutFileType -> SdkConstants.FD_RES_LAYOUT to "layout.xml"
       PreferenceScreenFileType -> SdkConstants.FD_RES_XML to "preference.xml"
-      MenuFileType -> SdkConstants.FD_RES_XML to "menu.xml"
+      MenuFileType -> SdkConstants.FD_RES_MENU to "menu.xml"
       else -> error("unknown type")
     }
     myModel = createModel(resourceFolder, name).build()