Implement GWT Focusable so tab indexes work correctly in MenuBar (#9733)

Fixes #9550

client/src/main/java/com/vaadin/client/ui/SimpleFocusablePanel.java

@@ -30,9 +30,9 @@ import com.google.gwt.event.dom.client.KeyPressHandler;
 import com.google.gwt.event.dom.client.KeyUpEvent;
 import com.google.gwt.event.dom.client.KeyUpHandler;
 import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.user.client.ui.Focusable;
 import com.google.gwt.user.client.ui.SimplePanel;
 import com.google.gwt.user.client.ui.impl.FocusImpl;
-import com.vaadin.client.Focusable;
 
 /**
  * Compared to FocusPanel in GWT this panel does not support eg. accesskeys, but
@@ -40,7 +40,7 @@ import com.vaadin.client.Focusable;
  */
 public class SimpleFocusablePanel extends SimplePanel
         implements HasFocusHandlers, HasBlurHandlers, HasKeyDownHandlers,
-        HasKeyPressHandlers, Focusable {
+        HasKeyPressHandlers, Focusable, com.vaadin.client.Focusable {
 
     public SimpleFocusablePanel() {
         // make focusable, as we don't need access key magic we don't need to
@@ -72,6 +72,7 @@ public class SimpleFocusablePanel extends SimplePanel
         return addDomHandler(handler, KeyUpEvent.getType());
     }
 
+    @Override
     public void setFocus(boolean focus) {
         if (focus) {
             FocusImpl.getFocusImplForPanel().focus(getElement());
@@ -85,7 +86,18 @@ public class SimpleFocusablePanel extends SimplePanel
         setFocus(true);
     }
 
+    @Override
     public void setTabIndex(int tabIndex) {
         getElement().setTabIndex(tabIndex);
     }
+
+    @Override
+    public int getTabIndex() {
+        return getElement().getTabIndex();
+    }
+
+    @Override
+    public void setAccessKey(char key) {
+        FocusUtil.setAccessKey(this, key);
+    }
 }

testbench-api/src/main/java/com/vaadin/testbench/elements/DateFieldElement.java

@@ -36,8 +36,7 @@ public class DateFieldElement extends AbstractDateFieldElement {
      */
     @Override
     public void clear() {
-        WebElement elem = findElement(By.tagName("input"));
-        elem.clear();
+        getInputElement().clear();
     }
 
     /**
@@ -46,7 +45,7 @@ public class DateFieldElement extends AbstractDateFieldElement {
      * @return value of the date field element
      */
     public String getValue() {
-        return findElement(By.tagName("input")).getAttribute("value");
+        return getInputElement().getAttribute("value");
     }
 
     /**
@@ -61,7 +60,7 @@ public class DateFieldElement extends AbstractDateFieldElement {
         if (isReadOnly()) {
             throw new ReadOnlyException();
         }
-        WebElement elem = findElement(By.tagName("input"));
+        WebElement elem = getInputElement();
         TestBenchElement tbElement = (TestBenchElement) elem;
         clearElementClientSide(tbElement);
         tbElement.sendKeys(chars);
@@ -109,4 +108,14 @@ public class DateFieldElement extends AbstractDateFieldElement {
         return DateTimeFormatter.ISO_LOCAL_DATE;
     }
 
+    /**
+     * Gets the {@code <input>} element inside the component.
+     *
+     * @return the input element inside the component
+     * @since
+     */
+    public WebElement getInputElement() {
+        return findElement(By.tagName("input"));
+    }
+
 }

testbench-api/src/main/java/com/vaadin/testbench/elements/InlineDateFieldElement.java

@@ -15,6 +15,9 @@
  */
 package com.vaadin.testbench.elements;
 
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
 import com.vaadin.testbench.elementsbase.ServerClass;
 
 /**
@@ -23,4 +26,14 @@ import com.vaadin.testbench.elementsbase.ServerClass;
 @ServerClass("com.vaadin.ui.InlineDateField")
 public class InlineDateFieldElement extends AbstractFieldElement {
 
+    /**
+     * Returns the element which receives focus when the component is focused.
+     *
+     * @return the element which receives focus when the component is focused
+     * @since
+     */
+    public WebElement getFocusElement() {
+        return findElement(By.tagName("table"));
+
+    }
 }

testbench-api/src/main/java/com/vaadin/testbench/elements/ListSelectElement.java

@@ -123,4 +123,14 @@ public class ListSelectElement extends AbstractSelectElement {
 
     }
 
+    /**
+     * Gets the {@code <select>} element inside the component.
+     *
+     * @return the select element inside the component
+     * @since
+     */
+    public WebElement getSelectElement() {
+        return selectElement;
+    }
+
 }

testbench-api/src/main/java/com/vaadin/testbench/elements/NativeSelectElement.java

@@ -17,6 +17,7 @@ package com.vaadin.testbench.elements;
 
 import java.util.List;
 
+import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ui.Select;
 
 import com.vaadin.testbench.By;
@@ -25,23 +26,35 @@ import com.vaadin.testbench.elementsbase.ServerClass;
 
 @ServerClass("com.vaadin.ui.NativeSelect")
 public class NativeSelectElement extends AbstractSelectElement {
-    private Select selectElement;
+    private Select select;
+    private WebElement selectElement;
 
     @Override
     protected void init() {
         super.init();
-        selectElement = new Select(findElement(By.tagName("select")));
+        selectElement = findElement(By.tagName("select"));
+        select = new Select(selectElement);
+    }
+
+    /**
+     * Gets the {@code <select>} element inside the component.
+     *
+     * @return the select element inside the component
+     * @since
+     */
+    public WebElement getSelectElement() {
+        return selectElement;
     }
 
     public List<TestBenchElement> getOptions() {
-        return wrapElements(selectElement.getOptions(), getCommandExecutor());
+        return wrapElements(select.getOptions(), getCommandExecutor());
     }
 
     public void selectByText(String text) throws ReadOnlyException {
         if (isReadOnly()) {
             throw new ReadOnlyException();
         }
-        selectElement.selectByVisibleText(text);
+        select.selectByVisibleText(text);
         waitForVaadin();
     }
 
@@ -55,16 +68,16 @@ public class NativeSelectElement extends AbstractSelectElement {
     }
 
     /**
-     * Return value of the selected item in the native select element
+     * Return value of the selected item in the native select element.
      *
      * @return value of the selected item in the native select element
      */
     public String getValue() {
-        return selectElement.getFirstSelectedOption().getText();
+        return select.getFirstSelectedOption().getText();
     }
 
     /**
-     * Select item of the native select element with the specified value
+     * Select item of the native select element with the specified value.
      *
      * @param chars
      *            value of the native select item will be selected

testbench-api/src/main/java/com/vaadin/testbench/elements/RichTextAreaElement.java

@@ -15,8 +15,22 @@
  */
 package com.vaadin.testbench.elements;
 
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
 import com.vaadin.testbench.elementsbase.ServerClass;
 
 @ServerClass("com.vaadin.ui.RichTextArea")
 public class RichTextAreaElement extends AbstractFieldElement {
+
+    /**
+     * Gets the {@code <iframe>} element inside the component, containing the
+     * editor.
+     *
+     * @return the iframe element containing the editor
+     * @since
+     */
+    public WebElement getEditorIframe() {
+        return findElement(By.tagName("iframe"));
+    }
 }

testbench-api/src/main/java/com/vaadin/testbench/elements/TwinColSelectElement.java

@@ -33,13 +33,19 @@ public class TwinColSelectElement extends AbstractSelectElement {
     private WebElement selButton;
     private static org.openqa.selenium.By bySelect = By.tagName("select");
     private static org.openqa.selenium.By byButton = By.className("v-button");
+    private WebElement optionsElement;
+    private WebElement selectionsElement;
 
     @Override
     protected void init() {
         super.init();
         List<WebElement> selectElements = findElements(bySelect);
-        options = new Select(selectElements.get(0));
-        selectedOptions = new Select(selectElements.get(1));
+
+        optionsElement = selectElements.get(0);
+        selectionsElement = selectElements.get(1);
+
+        options = new Select(optionsElement);
+        selectedOptions = new Select(selectionsElement);
         List<WebElement> buttons = findElements(byButton);
         selButton = buttons.get(0);
         deselButton = buttons.get(1);
@@ -154,4 +160,26 @@ public class TwinColSelectElement extends AbstractSelectElement {
     public void clear() {
         deselectAll();
     }
+
+    /**
+     * Gets the left {@code <select>} element inside the component, containing
+     * the available options.
+     *
+     * @return the select element containing options inside the component
+     * @since
+     */
+    public WebElement getOptionsElement() {
+        return optionsElement;
+    }
+
+    /**
+     * Gets the right {@code <select>} element inside the component, containing
+     * the selected options.
+     *
+     * @return the select element containing selection inside the component
+     * @since
+     */
+    public WebElement getSelectionsElement() {
+        return selectionsElement;
+    }
 }

uitest/src/main/java/com/vaadin/tests/fields/TabIndexes.java

@@ -3,37 +3,60 @@ package com.vaadin.tests.fields;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.CheckBox;
+import com.vaadin.ui.ComboBox;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.DateField;
-import com.vaadin.ui.GridLayout;
 import com.vaadin.ui.HorizontalLayout;
 import com.vaadin.ui.InlineDateField;
+import com.vaadin.ui.ListSelect;
+import com.vaadin.ui.MenuBar;
+import com.vaadin.ui.NativeSelect;
+import com.vaadin.ui.PasswordField;
 import com.vaadin.ui.RichTextArea;
 import com.vaadin.ui.Slider;
-import com.vaadin.v7.ui.ComboBox;
-import com.vaadin.v7.ui.ListSelect;
-import com.vaadin.v7.ui.NativeSelect;
-import com.vaadin.v7.ui.OptionGroup;
-import com.vaadin.v7.ui.PasswordField;
-import com.vaadin.v7.ui.Table;
-import com.vaadin.v7.ui.TextArea;
-import com.vaadin.v7.ui.TextField;
-import com.vaadin.v7.ui.Tree;
-import com.vaadin.v7.ui.TreeTable;
-import com.vaadin.v7.ui.TwinColSelect;
+import com.vaadin.ui.TextArea;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.TreeGrid;
+import com.vaadin.ui.TwinColSelect;
+import com.vaadin.ui.VerticalLayout;
 
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class TabIndexes extends AbstractTestUIWithLog {
 
-    private List<Focusable> fields;
+    public static final String FIELD_CONTAINER_ID = "field-container";
+    private List<Focusable> fields = new ArrayList<>();
 
     @Override
     protected void setup(VaadinRequest request) {
+        fields.add(new ComboBox());
+        fields.add(new NativeSelect());
+        fields.add(new ListSelect());
+        fields.add(new TextField());
+        fields.add(new DateField());
+        fields.add(new InlineDateField());
+        TreeGrid<String> tt = new TreeGrid<>();
+        tt.addColumn(s -> s);
+        tt.setItems("Foo", "Bar");
+
+        fields.add(tt);
+        fields.add(new TwinColSelect<String>());
+        fields.add(new PasswordField());
+        fields.add(new TextArea());
+        fields.add(new RichTextArea());
+        fields.add(new CheckBox());
+        fields.add(new Slider());
+        MenuBar menubar = new MenuBar();
+        menubar.addItem("foo", item -> {
+        });
+        fields.add(menubar);
+
         HorizontalLayout buttonLayout = new HorizontalLayout();
         addComponent(buttonLayout);
         Button clearTabIndexes = new Button("Set all tab indexes to 0");
@@ -87,51 +110,19 @@ public class TabIndexes extends AbstractTestUIWithLog {
                 updateCaptions();
             }
         });
-        fields = new ArrayList<>();
-        Table t = new Table();
-        t.setSelectable(true);
-        t.addContainerProperty("foo", String.class, "bar");
-        t.addItem();
-        fields.add(t);
-        fields.add(new ComboBox());
-        fields.add(new NativeSelect());
-        fields.add(new ListSelect());
-        fields.add(new TextField());
-        fields.add(new DateField());
-        fields.add(new InlineDateField());
-        OptionGroup og = new OptionGroup();
-        og.addItem("Item 1");
-        og.addItem("Item 2");
-        fields.add(og);
-        TreeTable tt = new TreeTable();
-        tt.setSelectable(true);
-        tt.addContainerProperty("foo", String.class, "bar");
-        tt.addItem();
-
-        fields.add(tt);
-        Tree tree = new Tree();
-        tree.addItem("Item 1");
-        fields.add(tree);
-        fields.add(new TwinColSelect());
-        fields.add(new PasswordField());
-        fields.add(new TextField());
-        fields.add(new TextArea());
-        fields.add(new RichTextArea());
-        fields.add(new CheckBox());
-        fields.add(new Slider());
 
         clearTabIndexes.click();
 
         buttonLayout.addComponents(clearTabIndexes, setTabIndexesToOne,
                 setTabIndexesInOrder, setTabIndexesInReverseOrder);
 
-        int fieldId = 1;
-        GridLayout gl = new GridLayout(4, 4);
+        VerticalLayout vl = new VerticalLayout();
+        vl.setId(FIELD_CONTAINER_ID);
         for (Component f : fields) {
-            f.setId("field-" + fieldId++);
-            gl.addComponent(f);
+            f.setId("field-" + f.getClass().getSimpleName());
+            vl.addComponent(f);
         }
-        addComponent(gl);
+        addComponent(vl);
 
     }
 

uitest/src/test/java/com/vaadin/tests/fields/TabIndexesTest.java

@@ -7,10 +7,26 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.elements.AbstractComponentElement;
 import com.vaadin.testbench.elements.ButtonElement;
-import com.vaadin.tests.tb3.MultiBrowserTest;
-
-public class TabIndexesTest extends MultiBrowserTest {
+import com.vaadin.testbench.elements.CheckBoxElement;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elements.DateFieldElement;
+import com.vaadin.testbench.elements.InlineDateFieldElement;
+import com.vaadin.testbench.elements.ListSelectElement;
+import com.vaadin.testbench.elements.MenuBarElement;
+import com.vaadin.testbench.elements.NativeSelectElement;
+import com.vaadin.testbench.elements.PasswordFieldElement;
+import com.vaadin.testbench.elements.RichTextAreaElement;
+import com.vaadin.testbench.elements.SliderElement;
+import com.vaadin.testbench.elements.TextAreaElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.testbench.elements.TreeGridElement;
+import com.vaadin.testbench.elements.TwinColSelectElement;
+import com.vaadin.testbench.elements.VerticalLayoutElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class TabIndexesTest extends SingleBrowserTest {
 
     @Override
     public void setup() throws Exception {
@@ -22,7 +38,7 @@ public class TabIndexesTest extends MultiBrowserTest {
     public void testTabIndexesSetToZero() {
         // clicked by default
         assertLogText("1. Setting tab indexes to 0");
-        for (WebElement element : getFieldElements()) {
+        for (WebElement element : getFocusElements()) {
             assertTabIndex("0", element);
         }
     }
@@ -30,7 +46,7 @@ public class TabIndexesTest extends MultiBrowserTest {
     @Test
     public void testTabIndexesSetToOne() {
         setTabIndexesTo("1");
-        for (WebElement element : getFieldElements()) {
+        for (WebElement element : getFocusElements()) {
             assertTabIndex("1", element);
         }
     }
@@ -39,7 +55,7 @@ public class TabIndexesTest extends MultiBrowserTest {
     public void testTabIndexesSetToOneThroughN() {
         setTabIndexesTo("1..N");
         int counter = 0;
-        for (WebElement element : getFieldElements()) {
+        for (WebElement element : getFocusElements()) {
             ++counter;
             assertTabIndex(String.valueOf(counter), element);
         }
@@ -48,7 +64,7 @@ public class TabIndexesTest extends MultiBrowserTest {
     @Test
     public void testTabIndexesSetToNThroughOne() {
         setTabIndexesTo("N..1");
-        List<WebElement> fieldElements = getFieldElements();
+        List<WebElement> fieldElements = getFocusElements();
         int counter = fieldElements.size();
         for (WebElement element : fieldElements) {
             assertTabIndex(String.valueOf(counter), element);
@@ -68,97 +84,42 @@ public class TabIndexesTest extends MultiBrowserTest {
     }
 
     private void assertTabIndex(String expected, WebElement element) {
-        Assert.assertEquals("Unexpected tab index,", expected,
-                element.getAttribute("tabIndex"));
-    }
-
-    private List<WebElement> getFieldElements() {
-        List<WebElement> fieldElements = new ArrayList<>();
-        fieldElements.add(getElement1());
-        fieldElements.add(getElement2());
-        fieldElements.add(getElement3());
-        fieldElements.add(getElement4());
-        fieldElements.add(getElement5());
-        fieldElements.add(getElement6());
-        fieldElements.add(getElement7());
-        fieldElements.add(getElement8());
-        fieldElements.add(getElement9());
-        fieldElements.add(getElement10());
-        fieldElements.add(getElement11());
-        fieldElements.add(getElement12());
-        fieldElements.add(getElement13());
-        fieldElements.add(getElement14());
-        fieldElements.add(getElement15());
-        fieldElements.add(getElement16());
-        fieldElements.add(getElement17());
-        return fieldElements;
-    }
-
-    private WebElement getElement1() {
-        return vaadinElement("PID_Sfield-1/domChild[1]/domChild[1]");
-    }
-
-    private WebElement getElement2() {
-        return vaadinElement("PID_Sfield-2/domChild[0]");
-    }
-
-    private WebElement getElement3() {
-        return vaadinElement("PID_Sfield-3/domChild[0]");
-    }
-
-    private WebElement getElement4() {
-        return vaadinElement("PID_Sfield-4/domChild[0]");
-    }
-
-    private WebElement getElement5() {
-        return vaadinElement("PID_Sfield-5");
-    }
-
-    private WebElement getElement6() {
-        return vaadinElement("PID_Sfield-6/domChild[0]");
-    }
-
-    private WebElement getElement7() {
-        return vaadinElement("PID_Sfield-7/domChild[0]");
-    }
-
-    private WebElement getElement8() {
-        return vaadinElement("PID_Sfield-8/domChild[0]/domChild[0]");
-    }
-
-    private WebElement getElement9() {
-        return vaadinElement("PID_Sfield-9/domChild[1]/domChild[1]");
-    }
-
-    private WebElement getElement10() {
-        return vaadinElement("PID_Sfield-10/domChild[1]");
-    }
-
-    private WebElement getElement11() {
-        return vaadinElement("PID_Sfield-11/domChild[1]");
-    }
-
-    private WebElement getElement12() {
-        return vaadinElement("PID_Sfield-12");
-    }
-
-    private WebElement getElement13() {
-        return vaadinElement("PID_Sfield-13");
-    }
-
-    private WebElement getElement14() {
-        return vaadinElement("PID_Sfield-14");
-    }
-
-    private WebElement getElement15() {
-        return vaadinElement("PID_Sfield-15/domChild[1]");
+        if (!expected.equals(element.getAttribute("tabIndex"))) {
+            Assert.assertEquals(
+                    "Unexpected tab index for element "
+                            + element.getAttribute("outerHTML"),
+                    expected, element.getAttribute("tabIndex"));
+        }
     }
 
-    private WebElement getElement16() {
-        return vaadinElement("PID_Sfield-16/domChild[0]");
+    private List<WebElement> getFocusElements() {
+        List<WebElement> focusElements = new ArrayList<>();
+
+        focusElements.add($(ComboBoxElement.class).first().getInputField());
+        focusElements
+                .add($(NativeSelectElement.class).first().getSelectElement());
+        focusElements
+                .add($(ListSelectElement.class).first().getSelectElement());
+        focusElements.add($(TextFieldElement.class).first());
+        focusElements.add($(DateFieldElement.class).first().getInputElement());
+        focusElements
+                .add($(InlineDateFieldElement.class).first().getFocusElement());
+        focusElements.add($(TreeGridElement.class).first());
+        focusElements
+                .add($(TwinColSelectElement.class).first().getOptionsElement());
+        focusElements.add($(PasswordFieldElement.class).first());
+        focusElements.add($(TextAreaElement.class).first());
+        focusElements
+                .add($(RichTextAreaElement.class).first().getEditorIframe());
+        focusElements.add($(CheckBoxElement.class).first().getInputElement());
+        focusElements.add($(SliderElement.class).first());
+        focusElements.add($(MenuBarElement.class).first());
+
+        List<AbstractComponentElement> components = $(
+                VerticalLayoutElement.class).id(TabIndexes.FIELD_CONTAINER_ID)
+                        .$(AbstractComponentElement.class).all();
+        Assert.assertEquals(components.size(), focusElements.size());
+        return focusElements;
     }
 
-    private WebElement getElement17() {
-        return vaadinElement("PID_Sfield-17");
-    }
 }