Fix incorrect system notifications with details styling (#18340)

Change-Id: Ia0d36147eb4ed9f170123771ac2674df584e6a4b

6 files changed, 54 insertions, 29 deletions

diff --git a/uitest/src/com/vaadin/tests/application/CriticalNotificationsTestBase.java b/uitest/src/com/vaadin/tests/application/CriticalNotificationsTestBase.java
index 2f32fa8026..f3813fce50 100644
--- a/uitest/src/com/vaadin/tests/application/CriticalNotificationsTestBase.java
+++ b/uitest/src/com/vaadin/tests/application/CriticalNotificationsTestBase.java
@@ -18,6 +18,8 @@ package com.vaadin.tests.application;
 import org.junit.Test;
 
 import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.CheckBoxElement;
+import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.tb3.MultiBrowserThemeTest;
 
 public abstract class CriticalNotificationsTestBase extends
@@ -64,23 +66,13 @@ public abstract class CriticalNotificationsTestBase extends
     }
 
     @Test
-    public void authenticationError() throws Exception {
-        testCriticalNotification("Authentication error");
-    }
-
-    @Test
-    public void communicationError() throws Exception {
-        testCriticalNotification("Communication error");
-    }
-
-    @Test
     public void internalError() throws Exception {
         testCriticalNotification("Internal error");
     }
 
     @Test
-    public void cookiesDisabled() throws Exception {
-        testCriticalNotification("Cookies disabled");
+    public void internalErrorDetails() throws Exception {
+        testCriticalNotification("Internal error", true);
     }
 
     @Test
@@ -93,13 +85,27 @@ public abstract class CriticalNotificationsTestBase extends
         testCriticalNotification("Session expired");
     }
 
+    @Test
+    public void sessionExpiredDetails() throws Exception {
+        testCriticalNotification("Session expired", true);
+    }
+
     private void testCriticalNotification(String buttonCaption)
             throws Exception {
+        testCriticalNotification(buttonCaption, false);
+    }
+
+    private void testCriticalNotification(String buttonCaption,
+            boolean withDetails) throws Exception {
         openTestURL(); // "theme=" + getTheme());
+        if (withDetails) {
+            click($(CheckBoxElement.class).caption("Include details").first());
+        }
         $(ButtonElement.class).caption(buttonCaption).first().click();
         // Give the notification some time to animate
         sleep(1000);
-        compareScreen("notification");
+        compareScreen($(NotificationElement.class).first(),
+                "systemnotification");
     }
 
     @Override
diff --git a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
index 5b05d1d50a..edcd07ee89 100644
--- a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
+++ b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
@@ -114,6 +114,15 @@ public abstract class AbstractTB3Test extends ParallelTest {
      */
     private static final int BROWSER_TIMEOUT_IN_MS = 30 * 1000;
 
+    protected static DesiredCapabilities PHANTOMJS2() {
+        DesiredCapabilities phantomjs2 = new VaadinBrowserFactory().create(
+                Browser.PHANTOMJS, "2");
+        // Hack for the test cluster
+        phantomjs2
+                .setCapability("phantomjs.binary.path", "/usr/bin/phantomjs2");
+        return phantomjs2;
+    }
+
     private boolean debug = false;
 
     private boolean push = false;
@@ -956,7 +965,12 @@ public abstract class AbstractTB3Test extends ParallelTest {
     }
 
     protected void click(CheckBoxElement checkbox) {
-        checkbox.findElement(By.xpath("input")).click();
+        WebElement cb = checkbox.findElement(By.xpath("input"));
+        if (BrowserUtil.isChrome(getDesiredCapabilities())) {
+            testBenchElement(cb).click(0, 0);
+        } else {
+            cb.click();
+        }
     }
 
     protected boolean isLoadingIndicatorVisible() {
diff --git a/uitest/src/com/vaadin/tests/tb3/MultiBrowserThemeTest.java b/uitest/src/com/vaadin/tests/tb3/MultiBrowserThemeTest.java
index 546588947c..0964e6eb65 100644
--- a/uitest/src/com/vaadin/tests/tb3/MultiBrowserThemeTest.java
+++ b/uitest/src/com/vaadin/tests/tb3/MultiBrowserThemeTest.java
@@ -17,8 +17,11 @@ package com.vaadin.tests.tb3;
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
+import org.openqa.selenium.remote.DesiredCapabilities;
+
 /**
  * Test which uses theme returned by {@link #getTheme()} for running the test
  */
@@ -27,9 +30,21 @@ public abstract class MultiBrowserThemeTest extends MultiBrowserTest {
     protected abstract String getTheme();
 
     @Override
+    protected boolean requireWindowFocusForIE() {
+        return true;
+    }
+
+    @Override
     protected void openTestURL(Class<?> uiClass, String... parameters) {
         Set<String> params = new HashSet<String>(Arrays.asList(parameters));
         params.add("theme=" + getTheme());
         super.openTestURL(uiClass, params.toArray(new String[params.size()]));
     }
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        List<DesiredCapabilities> browsersToTest = getBrowsersExcludingPhantomJS();
+        browsersToTest.add(PHANTOMJS2());
+        return browsersToTest;
+    }
 }
diff --git a/uitest/src/com/vaadin/tests/tb3/SingleBrowserTestPhantomJS2.java b/uitest/src/com/vaadin/tests/tb3/SingleBrowserTestPhantomJS2.java
index 9f4e5938e7..7447a3c056 100644
--- a/uitest/src/com/vaadin/tests/tb3/SingleBrowserTestPhantomJS2.java
+++ b/uitest/src/com/vaadin/tests/tb3/SingleBrowserTestPhantomJS2.java
@@ -20,15 +20,10 @@ import java.util.List;
 
 import org.openqa.selenium.remote.DesiredCapabilities;
 
-import com.vaadin.testbench.parallel.Browser;
-
 public abstract class SingleBrowserTestPhantomJS2 extends
         PrivateTB3Configuration {
     @Override
     public List<DesiredCapabilities> getBrowsersToTest() {
-        DesiredCapabilities p2 = Browser.PHANTOMJS.getDesiredCapabilities();
-        p2.setVersion("2");
-        p2.setCapability("phantomjs.binary.path", "/usr/bin/phantomjs2");
-        return Collections.singletonList(p2);
+        return Collections.singletonList(PHANTOMJS2());
     }
 }
diff --git a/uitest/src/com/vaadin/tests/tb3/VaadinBrowserFactory.java b/uitest/src/com/vaadin/tests/tb3/VaadinBrowserFactory.java
index f55b434ee0..294fe440d0 100644
--- a/uitest/src/com/vaadin/tests/tb3/VaadinBrowserFactory.java
+++ b/uitest/src/com/vaadin/tests/tb3/VaadinBrowserFactory.java
@@ -55,6 +55,8 @@ public class VaadinBrowserFactory extends DefaultBrowserFactory {
 
     @Override
     public DesiredCapabilities create(Browser browser, String version) {
-        return create(browser);
+        DesiredCapabilities capabilities = create(browser);
+        capabilities.setVersion(version);
+        return capabilities;
     }
 }
diff --git a/uitest/src/com/vaadin/tests/themes/valo/ValoThemeUITest.java b/uitest/src/com/vaadin/tests/themes/valo/ValoThemeUITest.java
index 4ed7e33e13..3c28b1f9eb 100644
--- a/uitest/src/com/vaadin/tests/themes/valo/ValoThemeUITest.java
+++ b/uitest/src/com/vaadin/tests/themes/valo/ValoThemeUITest.java
@@ -24,7 +24,6 @@ import com.vaadin.testbench.elements.CheckBoxElement;
 import com.vaadin.testbench.elements.CssLayoutElement;
 import com.vaadin.testbench.elements.LabelElement;
 import com.vaadin.testbench.elements.TreeElement;
-import com.vaadin.testbench.parallel.BrowserUtil;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
 public class ValoThemeUITest extends MultiBrowserTest {
@@ -204,13 +203,7 @@ public class ValoThemeUITest extends MultiBrowserTest {
      * workaround for http://dev.vaadin.com/ticket/13763
      */
     private void check(String caption) {
-        WebElement cb = $(CheckBoxElement.class).caption(caption).first()
-                .findElement(By.xpath("input"));
-        if (BrowserUtil.isChrome(getDesiredCapabilities())) {
-            testBenchElement(cb).click(0, 0);
-        } else {
-            cb.click();
-        }
+        click($(CheckBoxElement.class).caption(caption).first());
     }
 
     @Test