Start making the SafariOptions w3c safe

SeleniumHQ · Jul 13, 2018 · b99fa0d · b99fa0d
1 parent 0efc0be
commit b99fa0d
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 10 deletions.
diff --git a/java/client/src/org/openqa/selenium/safari/SafariOptions.java b/java/client/src/org/openqa/selenium/safari/SafariOptions.java
@@ -56,6 +56,7 @@ public class SafariOptions extends MutableCapabilities {
 
   private interface Option {
     String TECHNOLOGY_PREVIEW = "technologyPreview";
+    String TECH_PREVIEW = "se:safari:techPreview";
   }
 
   private Map<String, Object> options = new TreeMap<>();
@@ -120,13 +121,15 @@ public static SafariOptions fromCapabilities(Capabilities capabilities)
    */
   public SafariOptions setUseTechnologyPreview(boolean useTechnologyPreview) {
     options.put(Option.TECHNOLOGY_PREVIEW, useTechnologyPreview);
-    setCapability(BROWSER_NAME, useTechnologyPreview ? "Safari Technology Preview" : "safari");
+    // Use an object here, rather than a boolean to avoid a stack overflow
+    super.setCapability(Option.TECH_PREVIEW, Boolean.valueOf(useTechnologyPreview));
+    super.setCapability(BROWSER_NAME, useTechnologyPreview ? "Safari Technology Preview" : "safari");
     return this;
   }
 
   @Override
   public void setCapability(String key, Object value) {
-    if (Option.TECHNOLOGY_PREVIEW.equals(key)) {
+    if (Option.TECHNOLOGY_PREVIEW.equals(key) || Option.TECH_PREVIEW.equals(key)) {
       setUseTechnologyPreview(Boolean.valueOf(value.toString()));
     } else {
       super.setCapability(key, value);
@@ -135,7 +138,7 @@ public void setCapability(String key, Object value) {
 
   @Override
   public void setCapability(String key, boolean value) {
-    if (Option.TECHNOLOGY_PREVIEW.equals(key)) {
+    if (Option.TECHNOLOGY_PREVIEW.equals(key) || Option.TECH_PREVIEW.equals(key)) {
       setUseTechnologyPreview(value);
     } else {
       super.setCapability(key, value);
@@ -150,7 +153,7 @@ public SafariOptions setProxy(Proxy proxy) {
   // Getters
 
   public boolean getUseTechnologyPreview() {
-    return (boolean) options.getOrDefault(Option.TECHNOLOGY_PREVIEW, false);
+    return is(Option.TECH_PREVIEW)|| options.get(Option.TECHNOLOGY_PREVIEW) == Boolean.TRUE;
   }
 
   // (De)serialization of the options
@@ -163,9 +166,9 @@ public boolean getUseTechnologyPreview() {
   private static SafariOptions fromJsonMap(Map<?, ?> options)  {
     SafariOptions safariOptions = new SafariOptions();
 
-    Boolean useTechnologyPreview = (Boolean) options.get(Option.TECHNOLOGY_PREVIEW);
-    if (useTechnologyPreview != null) {
-      safariOptions.setUseTechnologyPreview(useTechnologyPreview);
+    Object useTechnologyPreview = options.get(Option.TECHNOLOGY_PREVIEW);
+    if (useTechnologyPreview instanceof Boolean) {
+      safariOptions.setUseTechnologyPreview((Boolean) useTechnologyPreview);
     }
 
     return safariOptions;

diff --git a/java/client/test/org/openqa/selenium/safari/SafariOptionsTest.java b/java/client/test/org/openqa/selenium/safari/SafariOptionsTest.java
@@ -18,11 +18,17 @@
 package org.openqa.selenium.safari;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import com.google.common.collect.ImmutableMap;
+
 import org.junit.Test;
 import org.openqa.selenium.ImmutableCapabilities;
 
+import java.util.HashMap;
+import java.util.Map;
+
 public class SafariOptionsTest {
 
   @Test
@@ -38,8 +44,31 @@ public void roundTrippingToCapabilitiesAndBackWorks() {
 
   @Test
   public void canConstructFromCapabilities() {
-    SafariOptions options = new SafariOptions(
-        new ImmutableCapabilities("technologyPreview", true));
+    Map<String, Object> embeddedOptions = new HashMap<>();
+    embeddedOptions.put("technologyPreview", true);
+
+    SafariOptions options = new SafariOptions();
+    assertFalse(options.getUseTechnologyPreview());
+
+    options = new SafariOptions(new ImmutableCapabilities(SafariOptions.CAPABILITY, embeddedOptions));
+    assertTrue(options.getUseTechnologyPreview());
+
+    embeddedOptions.put("technologyPreview", false);
+    options = new SafariOptions(new ImmutableCapabilities(SafariOptions.CAPABILITY, embeddedOptions));
+    assertFalse(options.getUseTechnologyPreview());
+
+    options = new SafariOptions(new ImmutableCapabilities("se:safari:techPreview", true));
+    assertTrue(options.getUseTechnologyPreview());
+
+    options = new SafariOptions(new ImmutableCapabilities("se:safari:techPreview", false));
+    assertFalse(options.getUseTechnologyPreview());
+  }
+
+  @Test
+  public void newerStyleCapabilityWinsOverOlderStyle() {
+    SafariOptions options = new SafariOptions(new ImmutableCapabilities(
+        SafariOptions.CAPABILITY, ImmutableMap.of("technologyPreview", false),
+        "se:safari:techPreview", true));
 
     assertTrue(options.getUseTechnologyPreview());
   }
@@ -56,4 +85,3 @@ public void settingTechnologyPreviewModeAlsoChangesBrowserName() {
     assertEquals("safari", options.getBrowserName());
   }
 }
-